spec: ECSM affine ecall variant - #932
Open
nicole-graus wants to merge 118 commits into
Open
Conversation
* register missing in-chip soundness constraints * add underconstrained-chip soundness guard * add underconstrained-chip soundness regression tests * harden in_chip_constraint_count against unsigned-subtraction underflow * Revert beyond-spec SHIFT/DVRM constraints * Update stale comments and capacity estimate * Fix stale bus-interaction comments left by soundness fixes The IS_HALF/IS_HALFWORD senders added in this PR made several doc comments and one capacity hint inaccurate; update them to match. - dvrm.rs: module doc IS_HALF count ×16 -> ×20 (matches the function doc, which already said ×20 after n/d were added) - mul.rs: module + bus_interactions docs IS_HALF ×8 -> ×16 and note the lhs/rhs input range checks, not just lo/hi outputs - shift.rs: module doc "11 total" -> "15 total", add IS_HALFWORD (×4) to the sender list, and bump Vec::with_capacity(11) -> 15 - test_utils.rs: create_lt_air / create_mul_air now wire transition constraints, so their doc comments say "constraints and bus interactions" (matching create_shift_air / create_dvrm_air) --------- Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com>
* fix infra LLVM toolchain for RISC-V asm * address provisioning review feedback
* Add shrink-cpu buses, decode layout, EQ chip * Add BYTEWISE ALU chip * Add BYTEWISE, STORE and CPU32 chips * Add CPU32 buses; fix EQ ALU output width * Register EQ/BYTEWISE/STORE/CPU32 as empty tables * Migrate CPU + ALU/memory chips to unified ALU bus * Delegate word instructions to the CPU32 table * Re-enable and rewrite CPU/decode/constraint tests for the shrink-cpu layout, and document the deviations * remove unnecesary files * Unify LT/MUL/memw/dvrm onto the ALU bus * Pin JALR rvd to pc+len * Align SHIFT shift-amount layout with the spec * reconcile prover with shrink-cpu spec * Add spec assumption range checks * Add explicit IS_BYTE[shift[0]] range check * Sync CPU with merged shrink-cpu spec * remove old comments * use constants * Propagate carry in branch rvd constraint * prevent register side effects in CPU32 padding * Use unreachable for validated carry arms * Force signed to zero on CPU32 padding rows * Force res_sign to zero on CPU32 padding rows * Gate CPU32 sign lookups by signed * close LT/SHIFT/LOAD underconstrained gaps * Remove dead LT carry helper and fix stale comments * Remove legacy byte-op buses --------- Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com> Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
* Add ECSM buses, cpu detection and EC_SCALAR * fix lint * Add ECSM and ECDAS accelerator tables * Register ECSM tables in VmAirs and trace builde * Wire ECSM ecall collection and end-to-end prove/verify test * Add ECSM soundness and multi-scalar end-to-end tests * Add IS_BIT(op) hardening to ECDAS and fix xR_sub_p comment * Add ecsm_mul guest wrapper + Rust guest and spec-bug regression tests * fix nits * Update root Cargo.lock and run ecsm tests in CI * Fix verifier sub-proof count for the 3 ECSM tables * Reject aliased xG/k and non-canonical xG in ECSM * Align ECSM/ECDAS AIR to the fixed spec * Delegate ecsm curve arithmetic to the RustCrypto k256 crate * Drop dead Fp methods and trim ecsm comments * feat(ecsm): k256-backed witness generation (projective + batch inverse) Replaces the per-operation Fermat inversions in the double-and-add replay with audited k256 (RustCrypto) projective arithmetic + batched inversion. The witness generator is untrusted (the ECDAS chip re-proves every step), so audited host-side arithmetic is sound here. - curve.rs: `replay_double_and_add` now replays the schedule in k256 `ProjectivePoint` (no per-op inversion), `batch_normalize`s every point to affine in one shot, and batch-inverts the slope denominators — two batched inversions instead of ~2·len_k Fermat modpows. The slope `λ` is precomputed here (new `StepPts.lambda` field) so the witness builder never inverts. - lib.rs: `scalar_mul_x` (executor) uses k256's optimized scalar mul directly, skipping the step list entirely. - witness.rs: `build_step` consumes the precomputed `s.lambda`. - The BigUint reference (`point_double`/`point_add`/`step_lambda`/ `replay_double_and_add_reference`) is kept `#[cfg(test)]` only — production ships k256 alone — and a parity test pins k256 == reference byte-for-byte across small/structured/large/near-order scalars. k256 is host-side only (witness gen), never in the constraint system, and was already a transitive workspace dependency. Replay micro-bench: ~5.9x faster than the BigUint reference on a 256-bit scalar. Follow-up (separate stage): port the field/curve primitives we need to drop the num-bigint reference path entirely. * Align ecsm docs, add executor/replay parity test * address review * move tests to correct directory * Clean up ECSM review nits * Fix prover clippy lints * Align ECSM/ECDAS/EC_SCALAR AIR to the rebased spec * renumber ecall * Fix the ECSM xR/xG address bound from +24 to +31 * Assert and document ECSM/ECDAS carry bounds * Add an ECSM benchmark guest program * Use mask and shift for the witness carry by 256 --------- Co-authored-by: diegokingston <dkingston@fi.uba.ar> Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com> Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
…e) (#671) * Add manual AI review tiers * Clarify standard AI review seriousness * Move AI review prompts into shared files * Clarify AI review is not a spec audit * Refine AI review prompt scope * Allow useful cosmetic AI review findings * Add orchestrated AI review matrix * Add AI review label triggers * Make AI review lanes resilient to provider failures * Harden AI review response parsing * Allow longer AI review model calls * Stop forcing JSON mode in AI review lanes response_format={type: json_object} was added in the hardening commit and turned out to be the cause of empty model responses: it routes to structured-output providers and makes reasoning models (minimax-m3, glm, mimo) reason until truncated at max_tokens without ever emitting content (observed reasoning_tokens=33989, completion_tokens=32000, findings=0). Make response_format opt-in per lane and rely on the existing extract_json parser, matching the request shape that works locally. Also capture finish_reason in the lane result so truncation is visible in the report. * Recover malformed model JSON with json-repair fallback Without forced JSON mode the model occasionally emits invalid JSON (e.g. unescaped quotes when a finding quotes code), which strict json.loads rejects all-or-nothing, dropping a whole review to zero findings. Add an optional json-repair fallback in extract_json: try strict parsing first, and only on failure fall back to repair, flagging it as a parse warning so invalid output stays visible. Install json-repair in the review/verifier lane steps. Verified against real lane output: recovers all 6 findings that strict parsing dropped. * Pin MiniMax lanes to healthy providers and shorten lane timeout Unpinned routing load-balances across the three cheapest minimax-m3 providers, one of which (Parasail) is currently deranked (status -2, ~94% uptime) and is the likely cause of a 15-minute lane hang. Pin the minimax-m3 lanes to the official Minimax provider then Novita (same $0.30/$1.20 price, ~99.6% uptime), ignore Parasail, and keep fallback to other healthy providers. Capture the serving provider in the lane result for diagnosis. Drop the lane timeout 900s -> 600s: healthy providers answer in well under a minute, and 600s still covers a full max_tokens response. * Pin MiniMax lanes to Novita only The official Minimax OpenRouter endpoint runs minimax-m3 at non-terminating reasoning effort: it enumerates 160+ trivial checks and never emits content, burning the full token budget on reasoning (34k+ reasoning tokens, empty output, finish_reason=length). It also ignores reasoning.max_tokens, so it cannot be capped. Novita serves the same model with a converging config (<=12k reasoning, produces findings), verified locally against the real PR context. Pin Novita exclusively; drop Minimax even as a fallback. * Retry OpenRouter calls on empty/transient responses A lane failed with 'OpenRouter request failed: Expecting value' because openrouter_chat did a single json.loads on the raw body with no retry. On slower requests OpenRouter emits SSE keep-alive comment lines and can return a whitespace-only body before the JSON arrives, which is transient. Wrap the request in a 3-attempt retry: strip SSE comment lines, treat an empty/whitespace body, a JSON decode failure, a network error, or a 5xx as retryable; non-retryable HTTP (e.g. 400) still fails fast. Legitimate empty message.content remains a non-retried error. Split parsing into parse_openrouter_response. * Make AI review lanes agentic via opencode Replace the single-shot OpenRouter chat lanes with agentic opencode runs so reviewers can explore the repository (read unchanged files, definitions, specs) instead of judging a stuffed diff blob. This fixes both the runaway reasoning (models nit-scanned a giant blob) and the inability to audit code that spans files. - .opencode/agent/review-ro.md: read-only sandbox agent (read/grep/glob/lsp only; bash, edit, write, patch, webfetch, websearch denied). With no shell the agent cannot read env vars or exfiltrate the API keys. - ai_review.py: new agentic-lane subcommand shells out to opencode, captures the final message, and reuses extract_json + json-repair to emit the same lane-result schema the candidates/verify/report pipeline already consumes. - workflow: review and verifier jobs now check out the PR, copy the trusted .opencode config from the runner branch (so a PR cannot weaken the sandbox), install opencode, and run with contents:read only (no GITHUB_TOKEN, no write) plus harden-runner egress audit. - matrix: agentic lineup of confirmed tool-driving models — nemotron-ultra, glm-5.1, deepseek-v4-pro as finders, with a smart verifier per tier. Small models (minimax, nemotron-super) do not drive the tool loop and were dropped. - Fix build_final_issues: conflicting verifier verdicts now resolve to 'uncertain' instead of silently 'confirmed' (found by glm and nemotron). - Stamp tier onto each lane in prepare so lane results classify correctly. * Fix opencode agent discovery in CI The first agentic run silently degraded: CI installed a newer opencode whose project-agent discovery dropped review-ro and which crashed on session-title generation, so lanes reported 'Agent not found: review-ro' and produced zero findings while the job still went green. - Pin opencode to 1.16.2 (the validated version; newer builds changed agent discovery and crash on title generation in CI). - Install the trusted review-ro agent globally (~/.config/opencode/agent) so it is discovered regardless of working directory or version. - Strip any PR-provided .opencode/opencode.json from the checkout so a PR cannot weaken its own sandbox. Validated locally: with no project .opencode, opencode 1.16.2 runs review-ro (15 tool-uses, 5 findings, no agent-not-found). * Raise agent step budget so reviews finish The agentic lanes ran and explored (26-36 tool-uses, no agent-not-found) but were cut off mid-exploration by opencode's default step cap, which forces a text-only response before the agent emits its JSON findings (so json-repair salvaged nothing -> 0 findings). A 1300-line file read in chunks easily exceeds the default. Set steps: 120 on the review-ro agent (the AgentConfig 'steps' = max agentic iterations before forcing a text response) and instruct it to converge and stop re-reading. The 700s lane timeout remains the wall-clock backstop. * Capture opencode output via structured JSON stream The agents explored and emitted findings, but run_opencode_agent parsed opencode's human-rendered stdout, which drops the final assistant message in CI's non-TTY environment -> extract_json found no JSON -> 0 findings (it only worked locally because that stream flushed the trailing JSON). Use 'opencode run --format json' and parse the JSONL event stream: the assistant output (including the final findings JSON) arrives in 'text' events at part.text. Surface stderr/stdout diagnostics when the agent emits nothing. Validated locally with the global agent and no project config (mimicking CI): nemotron-ultra now returns 5 structured findings. * Wire Kimi K2.7-Code as reviewer, bump GLM to 5.2 Reviewer (both tiers) -> moonshotai/kimi-k2.7-code: coding-specialized, tool -capable, and ~30% more token-efficient than K2.6, which helps agentic convergence. Finder GLM lanes bumped z-ai/glm-5.1 -> glm-5.2 (newer, 1M ctx). Finders remain a diverse tool-driving set (nemotron-ultra, glm-5.2, deepseek -v4-pro). * Support direct provider APIs and split into cheap/expensive tiers Generalize agentic lanes to any opencode provider: the lane 'model' is now a fully provider-qualified id (openrouter/..., minimax/MiniMax-M3, moonshotai/kimi-k2.7-code, anthropic/claude-opus-4-8, openai/gpt-5.5) and run_opencode_agent passes it through unchanged; opencode resolves credentials from env vars. Direct APIs avoid OpenRouter's tool-call mangling (which broke MiniMax/Kimi agentically). Lane jobs now export OPENROUTER/ANTHROPIC/OPENAI/MINIMAX/MOONSHOT keys (MOONSHOT_API_KEY fed from the KIMI_API_KEY secret). Tiers: - standard (cheap, no GPT/Claude): minimax + kimi + nemotron + glm-5.2 finders, deepseek verifier. - critical (expensive): adds deepseek + claude finders; claude finds, gpt verifies. * Deny write/patch in review-ro sandbox tools:{write:false} frontmatter was silently ignored (like doom_loop/steps), leaving the read-only review agent able to create files. Enforce via the permission block (which reliably applied edit/bash/webfetch denials). * Comparison grid: cheap models x 3 prompt variants; Claude broad Run every cheap finder (minimax, kimi, nemotron, glm) on the same 3 prompts (correctness, maintainability, tests) so models are comparable per-variant (model-metrics.json reports per-lane findings + unique candidates). Claude reviews everything via the broad 'general' prompt rather than a single concern. Soundness is no longer a default lane (most PRs don't touch constraints; reserved for a dedicated audit). Reviewers: deepseek (standard), gpt (critical). * Refine prompts: trim ZK-soundness, fold robustness into correctness, fuse quality+tests - general (Claude 'everything'): drop the ZK/prover-soundness bullet — models are weak at crypto and soundness is a separate audit; keep Rust safety + VM semantics + bugs/perf/simplicity. - correctness: absorb robustness (panics, overflow/underflow, OOB/off-by-one, unchecked casts) and this repo's real bug classes (byte/word packing, iteration-order nondeterminism affecting commitments). No separate robustness variant — it overlaps correctness. - quality: new lane fusing maintainability + tests (simplify/dedup/rename + coverage gaps); removes the standalone maintainability/tests prompts. - matrix: cheap grid is now 4 models x {correctness, quality} (8 lanes; +claude-general = 9 on critical). soundness.md retained for the future constraints tier. * Add GPU device-memory checks to correctness and general prompts CUDA code in this repo can OOM/leak and crash the whole run — the most common GPU failure. Add a conditional GPU/CUDA bullet (only fires when a PR touches GPU code): device-memory exhaustion/leaks, unbounded or growing allocations, buffers not freed, plus buffer lifetime and host/device synchronization. * Remove unused soundness prompt soundness.md is no longer referenced by the matrix (soundness was pulled from the default flow). The future constraints tier will get its own tightened, constraint-focused prompt. * Run agentic lanes in a single checkout (fix 0-findings) The lane jobs checked out the PR merge into both runner/ and subject/ (the default checkout is already the PR merge), so the agent saw two identical copies of every file, wandered between them, and exhausted its step budget before emitting findings — systematically across all models/lanes. Confirmed: locally with a single checkout the same model + same diff finds 8 issues; CI with the dual checkout found 0. Drop the redundant subject checkout and run opencode in the single runner tree (--repo runner). * Fix: keep context job on --repo subject (only lanes use runner) * Fix extract_json grabbing stray arrays before findings (root cause of 0 findings) json_has_required_shape treated ANY list as a match, so the JSON scan returned the first bare array it found (a code snippet in the agent's narration) and short-circuited BEFORE the json-repair fallback. Models often emit a malformed findings object (e.g. a stray quote) inside narration, so the real findings were never recovered -> 0 findings, systematically, across all lanes. Now extract_json collects all JSON candidates and prefers the LAST object that actually contains the required key; a stray scalar array is ignored, and when no clean object is found it falls through to json-repair (which recovers the malformed findings). Verified: a real lane raw that returned 0 now yields 10 findings with repair enabled. * Simplify finder grid to one prompt per model; add opencode stream diagnostics While experimenting, run a single prompt variant (correctness) per cheap model instead of both correctness+quality, halving finder lanes and noise. Add anti-narration directive to review-ro agent: opencode ends the turn on any no-tool-call message, so forbid planning-only replies and force the final JSON to be emitted immediately once exploration is done. Capture opencode stream event-type counts (tool_use/text/step_finish) and a stream tail per lane so we can tell a step-cap cutoff from a voluntary stop. * Resume opencode session to force final JSON when a lane ends empty Diagnostics from the standard run show the real failure: opencode ends the agent turn (reasoning/step budget) before the model emits its final JSON. glm spent a whole step on reasoning tokens (output:0, reasoning:6587) and died on the next step_start; minimax/nemotron narrated then stopped. It is not narration-vs-JSON and not a fixed step cap. When the first pass yields no parseable findings/verifications, capture the opencode session id and resume that session with a forcing prompt that demands ONLY the JSON (no more tools, no analysis) — the model keeps all the repo context it already explored. Continuation runs with a shorter timeout; the lane wrapper grows to 1100s to fit exploration + continuation. Also add --print-logs --log-level WARN so a silently-empty lane (kimi emits no stdout/stderr at all) surfaces its provider/auth cause next run. * Send opencode message on stdin to avoid E2BIG on large diffs The lane message (prompt + full PR diff) was passed as a single argv string. Once the diff crossed Linux MAX_ARG_STRLEN (~128KB) every lane died with '[Errno 7] Argument list too long: opencode' before opencode even started — which is why adding a few lines to ai_review.py suddenly broke all lanes. opencode reads the run message from stdin when no positional message is given, and stdin has no such size limit, so deliver the message there instead. * Capture opencode exit code and raise log level to INFO for lane diagnostics Lanes that die after a lone step_start report 'success' because the script ignores opencode's exit code and just parses the partial stdout. Record proc.returncode (137 would mean SIGKILL/OOM, non-zero a crash) and switch --print-logs to INFO so the failing lane's actual cause lands in stderr. The model itself is fine: glm-5.2 produces a correct single-step answer locally with the full 131KB diff, and three OpenRouter models run cleanly in parallel — so this is a CI-environment failure, not model/diff-size/rate-limit. * Cap lane reasoning effort with --variant low to stop empty/timeout turns Root cause of the empty lanes: glm/minimax are reasoning models that spend the whole turn on reasoning tokens, then emit empty output (glm: 5.4min on the model call, exit 0, only step_start) or time out (minimax >700s). Locally glm with --variant low produces a clean real finding in 264s instead; nemotron (223s) and deepseek (401s) also stay well under budget and emit valid JSON. Add an optional per-lane "variant" field, thread it into both the exploration and continuation opencode calls, and set variant=low on the cheap reasoning finders + DeepSeek verifier. Claude/GPT left at default (untested, not in the standard run). Kimi remains a separate 401 (Moonshot rejects KIMI_API_KEY); to be moved to OpenRouter later. * TEMP: probe direct-provider reliability with small Claude/GPT lanes Standard tier temporarily set to claude-haiku-4-5 + gpt-5-mini finders and a claude-haiku verifier (no variant) to test whether the direct Anthropic/OpenAI APIs reliably emit findings in CI — isolating whether the empty/500 failures are specific to the open-model providers/keys. Will restore the real standard grid after this run. * Restore open-model standard grid after direct-provider probe The probe confirmed the pipeline produces real, verified findings end-to-end with reliable providers (claude-haiku 4 findings, gpt-5-mini 2 via continuation, claude-haiku verifier). Restoring the cheap open-model standard grid; the open-model empties/500s/401 are a provider/key matter to resolve separately. * TEMP: cheap small-model standard tier + test OpenRouter key Swap standard finders to small/cheap OpenRouter models (gpt-5-nano, glm-4.7-flash, nemotron-3-nano) + gpt-5-nano verifier, and point the lane OpenRouter env at OPENROUTER_TEST_KEY (a disposable key) to test key-vs- environment cheaply. Will revert workflow + matrix and delete the test secret after this run. * TEMP: cheap open-weight standard tier + test OpenRouter key Standard finders = small/cheap open-weight OpenRouter models (qwen3-30b, glm-4.7-flash, nemotron-3-nano) + glm-4.7-flash verifier, with the lane OpenRouter env pointed at OPENROUTER_TEST_KEY (disposable) to test key-vs- environment cheaply. Will revert workflow + matrix and delete the test secret after this run. * TEMP: confirm glm-5.2 converges with healthy OpenRouter key Single glm-5.2 finder + glm-4.7-flash verifier on OPENROUTER_TEST_KEY, to confirm that the converging mid-size model produces findings once the key is healthy (org key died after 1 call; test key sustained dozens). Will revert + delete the test secret after. * Fire lane continuation on empty turns; revert test OpenRouter key When opencode ends a turn with no assistant text, the diagnostic fallback contains no findings-shaped JSON so parse_error could be None and the continuation never fired. Track meta.no_assistant_text and retry whenever a lane yields no items and either parsing failed or there was no assistant text. Revert the lane OpenRouter env back to secrets.OPENROUTER_API_KEY (the disposable test key is removed; deleting the secret next). * Restore intended open-model standard grid End of the open-model investigation: restore standard to the 4 cheap open finders (minimax, kimi, nemotron, glm; variant low) + deepseek verifier. These are known not to converge reliably in CI yet (reasoning models return empty output; flash/nano over-explore past the step cap) — tracked for later. The pipeline itself is proven correct with strong direct models. * Report review findings via a submit_findings tool, not free-text JSON The open models reliably make tool calls but routinely fail the final step we were asking for — stop exploring and hand-write a JSON blob (they emptied or wandered past the step cap). Replace that with a structured channel: - .opencode/tools/submit_findings.ts: schema-validated tool whose execute() writes findings to $AI_REVIEW_OUT (plugin code, so not gated by the agent permission block; opencode bundles @opencode-ai/plugin so no CI deps needed). - review lanes pre-create the output file with submitted:false (tri-state debug: no file = crashed early, submitted:false = tool never called, submitted:true = ran), set AI_REVIEW_OUT, tell the model to call submit_findings, and read it back. - end-injection: if the tool wasn't called, resume the session and force the call now (the ask is the current instruction, not a stale preamble). - de-blackbox: lane meta now carries a compact timeline (every tool call + args, text previews, per-step output/reasoning tokens) instead of just a truncated tail. - workflow installs custom tools globally alongside the agent. Verified locally end-to-end: glm-4.7-flash (which previously over-explored past the step cap) now calls submit_findings on the first pass and returns a valid finding. Verification lanes keep the text+continuation path for now. 25 tests pass. * TEMP: cheap open-model grid to validate submit_findings in CI Standard = glm-4.7-flash + nemotron-3-nano finders + glm-flash verifier (cheap, org OpenRouter key) to validate that the submit_findings tool makes the open models converge in CI on the real diff. Will set the final grid after. * Use absolute path for AI_REVIEW_OUT so the submit tool writes where the script reads CI validation showed glm-flash DID converge and call submit_findings (timeline confirms it), but submission came back submitted=false: AI_REVIEW_OUT was a relative path and opencode runs with a different cwd than ai_review.py (--repo points elsewhere), so the tool wrote to a non-existent runner/ai-review-lane dir (write failed, model then floundered). Resolve the path to absolute. * TEMP: glm-5.2 + variant low + submit tool — payoff validation glm-flash timed out (uncapped reasoning) and nemotron-nano was too weak. Use glm-5.2 (found real bugs locally) with variant low (bounds reasoning -> no timeout) and the submit_findings tool (avoids the free-text-JSON empties). Single finder + glm-flash verifier. Final grid TBD after this confirms. * Report verifications via a submit_verifications tool too Mirror the submit_findings approach for verifier lanes, which had the same free-text-JSON failure (produced 0 verifications -> findings stuck as 'candidate'). Add .opencode/tools/submit_verifications.ts, generalize read_submission(path, key) to return items for either findings or verifications, and route verifier lanes through the tool + end-injection. Drop the now-unused free-text continuation/schema constants. Verified locally: glm-4.7-flash calls submit_verifications and returns a structured verdict (rejected, with rationale). 25 tests pass. * TEMP: test minimax + submit_findings tool Retest minimax/MiniMax-M3 (variant low + submit_findings) — its old empties were the free-text-JSON failure the tool now fixes, not a confirmed bad key (no 401, unlike kimi). Single minimax finder + glm-flash verifier. * Production swarm: kimi via OpenRouter, full open-model finders + tool All open models now converge via submit_findings/submit_verifications. Route kimi through OpenRouter (openrouter/moonshotai/kimi-k2.7-code) to use the working OPENROUTER_API_KEY instead of the rejected direct Moonshot key (401). standard = glm-5.2 + minimax + kimi + nemotron finders (variant low + tool) + deepseek verifier. critical adds the Claude finder + GPT verifier. * Loosen finder prompt (report candidates w/ confidence), raise timeout, minimax thinking sweep minimax found 0 but its trace shows it SURFACED real candidates (the args.out relative-path bug, a git_file_text head-ref concern) then self-censored to 'high-confidence only' — the prompt was telling it to. That fights the finders-cast-wide -> verifier-filters design. - correctness.md / review-ro.md / SUBMIT_INSTRUCTION: report every plausible issue with an honest confidence; don't drop uncertain-but-real concerns (the verifier re-checks). Keep 'don't fabricate baseless noise'. Also fix the stale 'emit JSON as final reply' text -> call the submit tool. - workflow: per-lane opencode timeout 700->1000s, wrapper 1100->1400s, so deeper-thinking lanes finish instead of timing out. - TEMP standard = minimax thinking sweep (variant low/high/max/default) to measure recall vs thinking. Critical unchanged. * Report format nits: drop Verified-by column, wrap source cell, list discarded issues - Single verifier made the per-row 'Verified by' column noise -> drop it; add one plain line noting the verifier and that the Status column is its verdict. - 'Found by' cell renders lane_id<br>model so the model wraps to its own line and the table fits on screen. - Replace the terse 'Rejected candidates: N' line with a collapsed 'Discarded candidates' section showing each rejected finding + the verifier's reason (full data still in the final-issues.json artifact). * Capture per-lane opencode cost + token totals in meta Sum step_finish cost and input/output/reasoning tokens across the stream so each lane's result records its actual spend (the timeline only had per-step output). Makes variant/model cost comparisons exact instead of sampled from the tail. * Normalize finding paths to repo-relative before dedup Multi-lane dedup merged nothing (61 findings -> 61 candidates) because opencode runs in the runner/ checkout, so the same file arrives as '.github/...', 'runner/.github/...', or an absolute '.../runner/.github/...' — and find_duplicate_group keys on file equality. clean_path now strips everything up to the checkout's 'runner/' segment so the same file collapses. (Necessary but not sufficient: reworded duplicates still score below the text-similarity threshold — separate follow-up.) * Production swarm: minimax as high + max lanes Per the thinking-sweep, minimax-high (diff-reasoning recall) and minimax-max (deep exploration) find largely disjoint issues, so run both. standard = glm-5.2 + minimax-high + minimax-max + kimi(OpenRouter) + nemotron, deepseek verifier; critical adds Claude finder + GPT verifier. * Add single-shot LLM dedup step (configurable per tier) after heuristic merge Catches reworded cross-lane duplicates the path+text heuristic misses. Runs in the candidates phase as one direct OpenRouter call (no agentic loop / no exploration needed): conservative prompt, rich findings (id/file/line/title/ claim), reasoning effort from the tier's deduper.variant, max_tokens 40k so reasoning doesn't truncate the answer. Failure is safe — any error keeps the heuristic candidates (worst case: residual dupes, never a lost finding). Single-shot JSON (not a tool) is deliberate: the finders needed a tool because their AGENTIC LOOP ended before emitting JSON; this is one request/one response where JSON is reliable, and an OpenRouter function-call would reintroduce the tool-calling mangling we left OpenRouter to avoid. extract_json + json-repair guard malformed output. Config: matrix.json per-tier "deduper": {model, variant}; prepare emits it, the candidates job passes --deduper and gets OPENROUTER_API_KEY. Default minimax-m3 low (won the conservative-precision A/B vs deepseek). TEMP standard = 2 minimax agents (high + max) to test dedup end-to-end. * Harden review-ro: explicit external_directory deny (block secret exfiltration) Prompt-injection threat: a malicious PR could try to make the agent read /proc/self/environ or credential files and leak provider keys via a finding in the public report. Verified opencode confines read to the project dir, but only via external_directory's 'ask' default (auto-rejected non-interactively). Make it an explicit deny so it's a hard block — which also survives --dangerously-skip-permissions (that flag only auto-approves rules not explicitly denied). Confirmed: read of /tmp and /etc/hosts now hard-rejected, no secret obtained. Combined with bash/webfetch deny, env-var keys are unreachable. * Sweep finder effort low vs high (glm/kimi/nemotron) with generous timeouts Measure each open model's recall at low vs high before committing the swarm's effort, since low provably misses findings (minimax: 5 vs 43). standard = 6 finder lanes (glm/kimi/nemotron x {low,high}) + glm-flash verifier + deduper. Raise per-call timeout 1000->1800s and wrapper 1400->2200s so high-effort lanes finish instead of timing out (esp. kimi, which timed out at 700s before). * Fix agent path resolution: review at workspace root, not a runner/ subdir The agent's file reads were failing because the repo was checked out into a runner/ subdir, so opencode's cwd was .../lambda_vm/runner/ but the agent built absolute paths against the workspace root (.../lambda_vm/.github/...) — a sibling that doesn't exist, then external_directory:deny hard-blocked it. Check the repo out at the workspace root in the two agent jobs (drop path: runner, run --repo .) so the agent's paths resolve to real files. clean_path: strip GITHUB_WORKSPACE prefix instead of pattern-matching 'runner/' (which now false-matches /home/runner and was flagged by the review itself). Other jobs (prepare/context/candidates/final-report) keep their runner/ checkout — they run the script, not the agent, so they're unaffected. * Finalize production matrix from sweep results Measured config (path fix verified; reads now resolve): - standard (cheap): glm + kimi + nemotron at low (sweep: all produce; high gave no gain for glm, failed nemotron, and only shallow nitpicks for kimi while low caught its critical) + minimax high+max (its measured sweet spot, finds disjoint things) -> deepseek-v4-pro verifier -> minimax-m3 deduper. - critical (expensive): same swarm + claude-opus finder + gpt-5.5 verifier. Both tiers: submit_findings/submit_verifications tools, conservative LLM dedup. * Docs: update to agentic architecture + capture experiment learnings Replace the stale OpenRouter-JSON-mode / one-MiniMax-lane description with the real opencode agentic design (submit_findings/submit_verifications tools, finders->dedup->verifier->report, current production matrix). Add sections that record what we learned: per-model reasoning-effort sweep (only minimax benefits from high; others best at low; high trades depth for breadth), an add-a-model playbook, and gotchas (stdin not argv, repo-root checkout, two-stage dedup + big max_tokens, OpenRouter daily cap, kimi via OpenRouter, the read-only sandbox + the install-from-PR open issue, diagnostics). * Simplify matrix: unify finders on the broad 'general' prompt All finders (both tiers) now use 'general' (correctness + cosmetic + perf in one pass) instead of the prover-scoped 'correctness' — adds the cosmetic dimension to the cheap tier and stops models scoping out non-prover PRs. Effort low everywhere except minimax (high, its measured sweet spot). Dropped minimax-max: exploration is already covered by glm/kimi/nemotron at low, and high is the proven minimax mode. Tiers now differ only by models (cheap swarm vs + Claude finder + GPT verifier). * Fix bugs the review found in ai_review.py (batch) - scoped_provider_env: each lane's subprocess gets only its own provider key, not all of them (least privilege / defense-in-depth on top of the sandbox). - timeout salvage: if the tool already submitted before the lane timed out, keep those findings/verifications instead of discarding the whole lane. - parse_name_status: guard rename/copy lines against IndexError on short output. - git_file_text: return (None, False) for zero budget, not ("", True) — empty string was treated as real content. - cmd_context: give each changed file an equal budget share (head/base) instead of halving 'remaining' per file, which front-loaded file 1 and starved the rest. - post_or_update_comment: coerce a None comment list to [] (empty body -> crash). - write_github_outputs: extend the heredoc delimiter until absent from the payload. - clean_path: only strip GITHUB_WORKSPACE on exact/'/'-boundary match, not siblings. - format_location: don't render 'file:0' for unknown/whole-file line. Tests added for each. 34 pass. * Fix comment-trigger ref, wire tests into CI, trim unused prompt, sync docs - pr_ai_review.yaml: agent jobs now checkout the explicit PR merge ref so the /ai-review *comment* trigger reviews the PR, not the default branch (the label trigger already did). Fixes the recurring confirmed 'reviews wrong branch' bug. - new pr_ai_review_tests.yaml: runs the ai_review.py unit tests on PRs touching the review scripts (the suite wasn't wired into CI). - remove quality.md: unused since finders unified on the broad 'general' prompt (which covers cosmetic/dedup/renames). correctness.md kept — it's the detailed prover-specific prompt, recoverable if general proves too shallow on prover PRs. - docs: matrix table updated to the simplified general-prompt config. * Remove unused correctness.md prompt All finders unified on 'general', so correctness.md (the prover-specific prompt) is no longer referenced by any lane or by prepare. Removing it for consistency with the quality.md trim; recoverable from git history if a prover-targeted lane is wanted later. Remaining prompts: general (finders), verify/verify-critical (verifiers), standard/critical (tier custom_prompt). * Consolidate critical tier: open-weight swarm + native flagship reviews Based on a measured critical run (PR #671): - Drop claude-opus-4-8 from the swarm. As an opencode finder it cost ~$1.05/run for a single unique low finding; everything else it flagged was also found by cheaper open models. The structured swarm is now open-weight only (glm/kimi/ nemotron/minimax), which is the only practical way to run those models uniformly. - Switch the critical verifier from gpt-5.5 to deepseek-v4-pro (the standard verifier), and verify-critical -> verify. The gpt verifier was ~$0.76/run (29% of swarm cost) and its soundness bar never fired (finders use 'general', no soundness candidates reach it). GPT's value already arrives via native Codex. The structured pipeline is now open-weight end-to-end and both tiers share one verifier + one verify prompt; verify-critical.md removed. - Native Claude review model sonnet -> opus. Opus moves out of the constrained swarm and into its full native harness (claude-code-action, 30 turns), where it has the best shot. On the measured run native sonnet posted nothing. - Native Codex stays: it found a high (matrix value -> shell interpolation) that the entire swarm missed, so it earns its cost as an independent pass. Net: critical = the standard open-weight pipeline + independent native Codex (GPT) and Claude (opus) reviews. Docs updated. * Collapse AI review to a single manual flow; retire main's auto reviewers The standard and critical matrix configs had become identical (the open-weight swarm + deepseek verifier), differing only by critical also running the native reviews. Collapse to one manually-triggered flow: - matrix.json: drop the standard tier; keep one config (key 'critical' retained for backward compatibility with the workflow's tier gate). - Triggers route everything to the one flow: /ai-review (with or without a legacy standard|critical arg), and any ai-review* label (incl. legacy ai-review-standard/-critical). Workflow label allowlist now includes plain 'ai-review'. Never auto-triggered on PR open. - Retire main's always-on per-model workflows pr_review_{claude,codex,kimi}.yaml. They duplicated the native Codex/Claude reviewers the flow already triggers, and ran automatically on every PR. NOTE: this also removes the ad-hoc /kimi, /codex, /claude comment commands that lived in those workflows. - Rename prompts/critical.md -> native-review.md (it is the brief for the native Codex/Claude reviews, not a tier prompt) and load it by fixed name; delete the dead standard.md (its content never reached any model). - Docs updated to the single-flow model. Also commits the previously-uncommitted pr_ai_review_tests.yaml (offline unit-test CI for ai_review.py) from the earlier session. * Use one generic prompt for swarm and native reviews; drop soundness brief native-review.md (the renamed critical.md) carried a soundness section that was just a topic list — it named soundness areas (Fiat-Shamir, commitments, AIR inclusion, witness-soundness) without describing what a soundness bug looks like, so it did not actually help a model find them. Real soundness bugs need counterexample reasoning and spec knowledge, not buzzword prompting; that work is deferred to dedicated tooling. - Native Codex/Claude reviews now use the same generic general.md as the swarm (prepare loads general.md as custom_prompt); native-review.md deleted. - Only two prompts remain: general.md (all reviewers) + lanes/verify.md. - Docs: 'what the review covers' rewritten to one generic prompt; added a Lessons entry that the soundness gap is deliberate. * Harden AI review against PR-controlled code/secrets (pwn-request) The lane jobs check out the PR merge ref and execute code from it (ai_review.py, .opencode tools, matrix, prompts) in steps holding all five provider secrets, and interpolate ${{ matrix.lane.id }} straight into shell. Two High findings (raised by both native Codex and the swarm). Restricting *who* can trigger does not fix it — the risk is *whose code* runs (a trusted member running /ai-review on an external PR executes that PR's code with the secrets). - prepare now refuses fork PRs (pr_is_from_fork: head repo != base repo). Only same-repo branches — which require write access — reach the secret-bearing, code-executing steps. Covers the issue_comment path (which has secrets on any PR); pull_request already withholds secrets from forks. - Validate lane ids against [A-Za-z0-9._-] in prepare, and pass matrix.lane.id via the LANE_ID env var instead of raw ${{ }} shell interpolation, closing the matrix->shell injection at both source and sink. - 5 new unit tests (fork detection incl. deleted-fork null repo; lane-id allow/deny). Docs security section rewritten. Residual (accepted): a write-access user can still run code with the secrets — within the existing trust boundary. Full base-trusted-checkout refactor is a documented future option. * Fix workflow validation failure: empty ${{ }} in run-block comment A previous commit put a literal ${{ }} inside a comment in the lane run blocks. GitHub evaluates expressions everywhere in a workflow file (including comments), and an empty ${{ }} is invalid -> startup_failure, so no run could be created (the label trigger silently produced nothing). Reword the comment to drop the token. * Gate fork PRs in the trusted workflow if, not PR-controlled code Codex (correctly) flagged that pr_is_from_fork() runs inside ai_review.py, which on the pull_request (label) arm is checked out FROM the PR merge commit — so a fork PR could replace prepare and bypass the gate, emitting should_run=true with arbitrary matrix outputs. The check was in the wrong (untrusted) layer for that arm. Fix: gate the pull_request arm in the workflow `if` using the trusted event context (head.repo.full_name == base.repo.full_name), evaluated before any checkout, so a fork PR's prepare job never starts. The issue_comment arm runs prepare from the default branch (trusted), so its pr_is_from_fork check is trustworthy there; the Python check stays as that arm's gate + defense-in-depth. Docs/comments updated to explain the layering. * Defense-in-depth from adversarial review (F1/F2/F4) An independent opus security review confirmed the pwn-request hole is closed but flagged hardening worth doing: - F1: the trusted same-repo gate was enforced in only one place (prepare.if); downstream jobs that hold provider secrets / the write token and run PR-controlled ai_review.py were protected only transitively. Replicate the same-repo if-gate on openrouter-review, candidates, openrouter-verify, and final-report so it is no longer a single point of failure. - F2: model-supplied finding text (claim/evidence/suggested_fix/title) is now HTML-escaped before going into the posted comment, preventing markup/link injection into the bot comment (md_escape routes through html_escape). - F4: submit_findings/submit_verifications refuse to write unless AI_REVIEW_OUT is the expected lane-*.submit.json basename. Skipped F6 (SHA-pinning the first-party org reusable workflows) — it mainly adds update-management friction for marginal benefit when the same org owns both repos. Tests pass (incl. existing fork/lane-id guards). * Docs: sync to single-flow reality; drop vestigial multi-prompt section - Replace the stale 'Multiple Prompts Versus One Prompt' section (and its per-model multi-prompt 'Initial policy' table listing models not in the matrix) with a short note: one generic general.md for all reviewers. - Add-a-model playbook: 'tier' -> review_lanes/verifier_lanes; 'run the tier' -> 'run the review'. - Update the example provenance lane ids to current ones (nemotron/glm/ deepseek-verifier instead of minimax-correctness/glm-standard/qwen-standard). - Document the operational caveat: native Claude + the /ai-review comment trigger only activate after merge to the default branch (claude-code-action's default-branch guard; issue_comment uses the default-branch workflow). * Single command UX: drop the standard/critical distinction from user-facing surface There is one flow, so the standard/critical naming was vestigial where users see it: - Docs: present a single `/ai-review` command and `ai-review` label; the old `/ai-review standard|critical` forms and `ai-review-standard/-critical` labels still work (tolerant parser + allowlist) but are no longer advertised as a choice. - Report title: `## AI Review (critical)` -> `## AI Review` (the marker stays `<!-- ai-review:critical -->`, invisible, so existing comments still update). - Created the canonical `ai-review` label. Parser, label allowlist, and the internal matrix key (`critical`) are unchanged — back-compat preserved, just not surfaced as two options. * Harden CI egress + pin opencode installer (reviewer #4, #5) #5: the opencode installer was fetched unpinned (curl|bash) and run in a step holding all provider secrets. Now fetch it to a file, verify a pinned sha256 (fail-closed if the script changes), then run it. #4: harden-runner egress-policy audit only logged egress. Switch the lane jobs to 'block' with an allowlist harvested from a real run's harden-runner audit (GitHub Actions infra, opencode install/binary/catalog at opencode.ai + *.github usercontent + models.dev, pip + npm, and the model APIs openrouter.ai + api.minimax.io). A compromised dep/installer can no longer exfiltrate to an arbitrary host. Trade-off: adding a new direct provider requires adding its host to allowed-endpoints, or that lane is blocked. Validating with a run next. * Delete dead single-shot lane path; prompt now flags dead code (reviewer #2, #3) The single-shot review/verify path is unreachable — the workflow only runs agentic-lane (+ prepare/context/candidates/lane-error/report). Remove it: - run-lane/verify-lane subparsers + dispatch, cmd_run_lane/cmd_verify_lane, run_review_lane/run_verifier_lane (-161 lines). openrouter_chat, lane_base_result, and infer_tier_from_lane stay (the deduper + agentic path + lane-error use them). - Drop the 5 tests that covered the dead path (they were inflating apparent coverage of code the workflow no longer runs). 34 tests remain, all live paths. - general.md now flags dead/unreachable code under simplicity, so future PRs get called out for it. Not adding agentic-path unit tests: cmd_agentic_lane shells out to opencode and is impractical to test in isolation; its parsing/salvage helpers (read_submission, extract_json, dedup) are already covered. * Pin json-repair with hashes; escape model location in detail code-spans High (reviewer): json-repair was pip-installed unpinned, then imported in the lane step that holds the provider keys — a hijacked release could run import-time code with the secrets. Pin it to ==0.61.0 with sha256 hashes via a requirements file + --require-hashes (pip only honors --hash there, not on the CLI; verified locally incl. a wrong-hash negative test). Same in both lane installs. Low (reviewer): format_location(issue) was interpolated raw inside markdown code-spans in two detail sections, and file comes from model/tool output — a backtick or newline could break out and inject markdown. Add format_location_code (strips backticks/newlines; HTML is already literal inside a code span) and use it at both sites. The table cell already used md_escape. * Remove dead code orphaned by the run_*_lane deletion The validation run's minimax lane (and the new dead-code prompt) caught leftovers from the earlier single-shot removal: format_review_prompt / format_verification_prompt were only called by the deleted run_*_lane, and format_changed_files / format_file_context only by those — all now dead. Removed the cluster (-81 lines) plus the now-unused textwrap import. Full unused-function scan confirms no remaining orphans; 34 tests pass. * Report opencode lane failures as errors, not silent empty successes (reviewer) cmd_agentic_lane left status=success when opencode failed (auth/outage/402/crash) but no findings were submitted — masking reviewer failures as 'success with 0 findings' (exactly what the OpenRouter 402 lanes did last run). Add opencode_failed() and, when nothing was submitted, mark the lane status=error if opencode reported a failure — either a non-zero exit OR an 'error' event (a 402 exits 0 but emits an error event, so the return-code check alone misses it). Applied to both the review and verify not-submitted branches; a valid submit_* result still keeps success. (The dead single-shot formatters the same review flagged were already removed in b7fb33a.) +1 test; 35 pass. * Restore DEDUP_SYSTEM (I deleted it) + clear dead config from review-triage Triaging ALL lane findings across the experiment runs surfaced a real regression I introduced: the dead-code commit b7fb33a swept away the module-level DEDUP_SYSTEM constant (it sat between format_file_context and the next def, so the 'delete to next def' boundary took it). llm_dedup_candidates references it inside a try/except Exception: return candidates, so every run NameError'd and silently returned candidates unchanged — the LLM dedup has been a no-op (this is the early '61 -> 61, merged nothing'). Restored the constant; added a regression test that fails if it's missing or the dedup no-ops. Also from the same triage: - Remove unused 'import urllib.parse' (dead import). - Remove MOONSHOT_API_KEY from the lane env — kimi goes via OpenRouter, the /kimi command was retired, so it was dead config (and an unstripped key). - Add a concurrency group (cancel-in-progress) so rapid re-triggers can't race and post duplicate report comments. 36 tests pass. Lesson: name-anchored 'delete to next def' is unsafe for module constants between functions — audited both dead-code commits; DEDUP_SYSTEM was the only collateral. * Fetch renamed-file base content from old_path (reviewer) cmd_context fetched base content using the new path, which doesn't exist at the base ref for a rename/copy — so renamed files silently lost their base-side context in the review. Use old_path for the base fetch when present. * Knock out the actionable tail from the review triage - id-token (OIDC) scoped to only the native Claude job (it's the only one that needs it); removed from workflow-wide permissions so the internal jobs don't carry it. Codex job gets contents/PR/issues write only. - post_or_update_comment now paginates all comment pages, so it finds the existing report on busy PRs (>100 comments) instead of posting a duplicate. - apply_dedup_clusters keeps the richest evidence/suggested_fix across merged duplicates instead of always discarding the others'. - clean_path tolerates a trailing slash in GITHUB_WORKSPACE. Deliberately left (design/graceful/rare, per review): scoped_provider_env unknown- provider fallback, cmd_context per-file budget (graceful + agent explores), extract_json fallback heuristic, parse_name_status git-quoting, submit-unset. * Remove the vestigial tier/critical concept — there is one flow After collapsing standard/critical into a single flow, 'critical' lingered as internal naming (matrix key, tier output, the tier=='critical' gate, job names, the comment marker). There are no tiers, so remove the concept entirely: - matrix.json flattened to {review_lanes, verifier_lanes, deduper} (no tier key). - prepare reads the flat matrix; parse_review_trigger returns the PR number (or None); parse_tier_command/label -> is_review_command/is_review_label (bool). - Drop tier from lane_base_result/build_candidates/build_final_issues, remove infer_tier_from_lane, and the comment marker is a fixed REVIEW_COMMENT_MARKER ('<!-- ai-review -->'), not tier-keyed. - Native jobs renamed codex-critical-review/claude-critical-review -> codex-review/claude-review and no longer gated on tier (they run on the one flow); dropped the tier workflow output + the tier in the artifact name. - The native-reviews note in the report now always shows. Note: the comment marker changed, so the next run posts a fresh report comment on #671 once (the old marker won't match); harmless. 36 tests pass. * Final triage fixes: all-reviewers-failed banner, dead mapping, least-privilege - Report shows a loud 'all N reviewers failed' banner when review lanes ran but none succeeded, instead of implying a clean PR (high finding). - Remove the now-dead moonshotai/ PROVIDER_KEYS mapping (MOONSHOT_API_KEY is gone and no lane uses that prefix). - Least-privilege: default workflow permissions are now read-only; only final-report (posts the comment) and the native review jobs request write/id-token. The internal prepare/context/candidates jobs no longer carry issues/PR write. * Address local review: consistent rename detection + context fork-gate From the local opus reviewers' findings: - name_status now uses --find-renames --find-copies, matching the diff body, so rename/copy detection is consistent (the copy branch in parse_name_status was otherwise unreachable, and a heavy-edit rename could mismatch the diff). - The context job now carries the same same-repo if-gate as the other downstream jobs (defense-in-depth consistency; it checks out and runs PR code). Reviewers found no critical/high regressions, no dangling tier refs, and verified the supply-chain pins (incl. json-repair hashes vs PyPI). Remaining notes: pin the native reusable workflows to a SHA (already tracked in the PR), and openrouter_chat is now deduper-only (harmless defensive generality). * Fix review-found doc staleness + align label gate From the in-flight run's findings (glm + Codex): - docs: drop the removed <tier> artifact-path segment; the matrix is flat now, not 'keyed critical for backward compatibility' (both stale after the de-tier). - workflow label gate: use startsWith(label, 'ai-review') instead of an exact-list contains(), matching is_review_label's prefix behavior in ai_review.py (the list rejected ad-hoc ai-review* labels the Python parser accepts). Noted (deferred): Codex flagged that a *partial* reviewer outage (some lane artifacts missing) isn't banner-flagged — only a total outage is; the per-lane Reviewer Lanes table still shows it. Fuller expected-vs-present check is a follow-up. * Apply the two worthwhile lows from the run; leave the rest - Drop the unnecessary getattr(args, 'deduper', None) -> args.deduper (the candidates subparser always defines it). Pure cleanup. - Pass the deduper JSON via a DEDUPER_JSON env var instead of single-quote shell interpolation, matching the LANE_JSON/LANE_ID pattern (defense-in-depth; the source is matrix.json so not exploitable, but consistent). Left by design/risk: scoped_provider_env unknown-provider (design), extract_json bare-JSON selection (tested fallback), cmd_lane_error context dep (edge), binary null-scan window (heuristic). The <tier> doc path was already fixed in 2dec6f4.
* add first cuda files * fmt * fix clippy * gpu 2nd part * feat(cuda): Round 1 GPU LDE+commit dispatch + device-resident handles * merge main * comments fix * Update crypto/stark/src/gpu_lde.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/stark/src/gpu_lde.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/stark/src/gpu_lde.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/stark/src/gpu_lde.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/stark/src/gpu_lde.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * address reviews * fix review comments * address doc comment suggestions * fix * Pass replay transcript to bus-balance call in verify_vm_minimal * Update crypto/math-cuda/src/device.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/math-cuda/src/device.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/math-cuda/src/device.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/math-cuda/src/device.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/math-cuda/src/lde.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/math-cuda/src/lde.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/math-cuda/src/lde.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/math-cuda/src/lde.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/math-cuda/src/lde.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * add pr3 code * fix comments * fix sync stream after D2H in merke.rs * fix comments * address review feedback * Update crypto/math-cuda/src/barycentric.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * Update crypto/math-cuda/src/barycentric.rs Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> * fix imports * cuda integration tests * address review feedback * batch invert kernels and parity test * DEEP composition kernel * fri * gpu lde * gpu_lde * fri * add tests * fix * fix comments * add integration tests * fix comments * refactor test * rm dead code, refactor * fix * rm doc * gpu batch inverse * fix * fallback test * fix_comments * cleanup * fmt * address comments * harden inv_denoms guard, fix scan kernel race * fix debug assert * cache index muls, rename denom_sign --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com> Co-authored-by: Gabriel Bosio <38794644+gabrielbosio@users.noreply.github.com> Co-authored-by: gabrielbosio <gabrielbosio95@gmail.com> Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com>
…680) The AI Review workflow triggers on issue_comment:created with concurrency.cancel-in-progress set unconditionally to true. The native claude-review job posts its report as a GitHub App comment (claude[bot]), and App-token comments fire issue_comment events (unlike github-actions[bot] comments, which GitHub suppresses from re-triggering workflows). Because GitHub evaluates concurrency before any job-level if:, that bot comment spawned a second run which skipped every job (prepare's if: is false for a non-/ai-review comment) yet still cancelled the original run mid-flight via the shared concurrency group. The slower OpenRouter matrix lanes (glm, kimi, nemotron) were killed while the fastest lane (minimax) had already finished, so the always()-gated final-report posted a partial report containing only minimax. Gate cancel-in-progress on the trigger being a genuine request (a label event or an /ai-review command comment). This preserves the original intent of cancelling duplicate requests while making non-command bot comments queue-and-skip instead of cancelling an in-flight review.
…rator (#666) * ethrex integration * update benchs * add README * fix ci * fix * fix(ethrex): reconcile KZG docs, clarify pin, wire empty fixture, mark temp tool (#678) Addresses review findings on PR #666: - Cargo.toml: the comment claimed the lambdavm feature provides KZG 'incl. kzg-rs', but the guest Cargo.lock has no kzg/c-kzg — KZG is NOT linked. Remove the false claim; state 0x0a is unsupported (consistent with main.rs) and clarify the dep is an immutable rev pin (not a moving branch). - main.rs: make the KZG note precise — blob txs still execute (stateless block execution doesn't verify blob proofs); only the point-eval precompile (0x0a) fails closed/reverts. - rust.rs: add test_ethrex_empty_block so the committed ethrex_empty_block.bin fixture (previously read by no test) exercises the 0-tx rkyv layout and the guest==host path; mirrors test_ethrex_simple_tx. - tooling/ethrex-fixtures: add a grep-able TODO marking the crate temporary (delete once ethrex-replay replaces it), and a .gitignore for the stray .ethrex-fixtures-tmp store dir + target/. - bench README: point at the canonical 'rev' in Cargo.toml, not the lockfile. * fix ethrex docs and fixture hygiene (#679) * Require ethrex fixture generator args (#681) * Refresh ethrex fixture checksums from regen target (#682) * Check ethrex fixture checksums in CI (#683) --------- Co-authored-by: MauroFab <maurotoscano2@gmail.com> Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com> Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
* fix ethrex bench * Provision bench sysroot in a user-writable dir * Guard SYSROOT_DIR rm -rf and harden provisioning * Clarify sysroot guard comment scope * Merge pull request #677 from yetanotherco/fix/sysroot-download-robustness Robustness + review fixes for nightly bench sysroot * Harden sysroot provisioning in provision.sh * Verify sysroot tarball before extraction (#684) --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com>
…ifier derives count from AIR (#699) * perf(stark): composition poly is the quotient — emit (d-1) parts, not d The composition polynomial is the constraint QUOTIENT H = Σ βᵢ·Cᵢ/Zᵢ, whose degree is (max_degree-1)·N (the end-exemptions of the max-degree LogUp constraints are 0). AirWithBuses::composition_poly_degree_bound returned trace_length·max_degree, committing+opening one extra all-zero part (3 parts for a degree-3 AIR where 2 suffice). Use (max_degree-1)·N. Effect on the VM (degree-3): 3 → 2 composition parts → one fewer LDE + Merkle commit + OOD opening per proof, and the degree-3 tables now take the fast algebraic decompose_and_extend_d2 path instead of the generic iFFT+break+FFT. Also: 2·(g·ωⁱ) via .double() instead of a base mul. Verifier needs no change: it derives the part count from the proof (verifier.rs:678/977), absorbs the parts into the transcript, and never calls composition_poly_degree_bound. Validated: stark 128/128 (AirWithBuses prove/verify), real VM proof (fib_iterative_1200k) prove+verify OK end-to-end, clippy + fmt clean. * fix(stark): verifier derives composition part count from the AIR (soundness) The verifier read the number of composition-poly parts from the proof (composition_poly_parts_ood_evaluation.len()). That count is a soundness parameter — it is fixed by the AIR's max constraint degree (composition_poly_degree_bound / trace_length). Trusting the proof let a malicious prover inflate the part count, widening the composition's degree space and weakening the low-degree test. multi_verify now derives the expected part count from the AIR and rejects any proof whose advertised count disagrees (+ a trace_length==0 guard). Adds a soundness test: an inflated part count is rejected. stark 129/129, clippy + fmt clean.
…set_main (#698) Table.data was a pub field, letting callers bypass the get/set accessors and poke the row-major buffer directly. That bypass is also a latent bug: under the disk-spill feature Table can be mmap-backed, and get/set handle that case while raw .data indexing does not -- keccak_rc's `main_table.data[..] = mu` would be wrong on a spilled table. Narrow Table.data to pub(crate) and route the one production write (keccak_rc multiplicity) through set_main, plus the handful of test reads through get_main. bitwise/decode already used the get/set API. Behavior-preserving: prover lib tests 416 pass (the 5 ecsm failures are pre-existing/environmental, identical on main), stark 128 pass; fmt + clippy clean. Supersedes #693 (2/2).
* Fix Msb16 LogUp over-send in MUL/DVRM * Address review: update collector docstrings and drop redundant import
…ommands (#704) * fix(ci): AI review — accept /review-ai alias, raise turn cap, scope agent commands - Accept both /ai-review and /review-ai as the trigger command (in the prepare gate, the concurrency cancel gate, and is_review_command), so a misremembered command no longer silently skips every job. - Raise claude-review max_turns 30 -> 50. Reviews die at 30 not because the work needs it (clean reviews finish in ~18-24 turns) but it leaves no headroom; 50 is a safety ceiling, not a budget. - Tell the native Codex/Claude agents (via general.md custom_prompt) which commands they may run and that they must not build/test/fetch or retry sandbox-denied commands. On PR #703 ~46% of Claude's tool calls were denied fetch/cargo/redirect attempts, exhausting the turn budget. * docs(ci): drop stale references to removed ai-review-standard/-critical labels
…tests/ (#688) * refactor(stark): move inlined tests + trace.rs test helpers into src/tests/ Part A — trace.rs: - Move `get_trace_evaluations` (Horner oracle) to tests/trace_test_helpers.rs - Move `compute_trace_polys_main` to tests/trace_test_helpers.rs as inherent impl - Widen `compute_frame_evaluation_points` to pub(crate) (needed by helper) - Remove all #[cfg(test)] items from trace.rs Part B — inline test modules: - grinding.rs → tests/grinding_tests.rs - bus_debug.rs → tests/bus_debug_tests.rs (gated: cfg(feature = "debug-checks")) - table.rs disk_spill_tests → tests/table_disk_spill_tests.rs (gated: cfg(feature = "disk-spill")) Visibility widened: - compute_frame_evaluation_points: fn → pub(crate) - BusDebugTracker.{bus_filter, logs}: private → pub(crate) - Table.mmap_backing: private → pub(crate) - TableMmapBacking: private struct → pub(crate) struct All 128 tests still pass; disk-spill feature adds 5 more (133 total). Clippy clean; production build clean. * fix(stark): import ParallelIterator in moved trace_test_helpers + cargo fmt The moved test helper used a rayon parallel iterator's .map() without ParallelIterator in scope (only IntoParallelRefIterator was imported), breaking the release test build under feature unification with the prover (which enables stark/parallel). Also applies cargo fmt to the moved tests.
#687) * refactor(ecsm): move inlined tests + reference arithmetic into src/tests/ The three source files mixed production code with test-only code. Relocate it all into a dedicated src/tests/ tree (matching the stark/prover convention: `#[cfg(test)] mod tests;` + tests/mod.rs), leaving lib.rs/curve.rs/witness.rs as pure production: - tests/lib_tests.rs <- lib.rs's inlined `mod tests` - tests/curve_tests.rs <- curve.rs's `mod parity_tests` - tests/witness_tests.rs <- witness.rs's inlined `mod tests` - tests/reference.rs <- curve.rs's #[cfg(test)] reference impl (point_double / point_add / step_lambda / replay_double_and_add_reference) - tests/reference_field.rs <- the whole #[cfg(test)] field.rs (BigUint Fp) - field.rs deleted Test helpers' hex-parse .unwrap() -> .expect(...) for clearer panics. Fixed a now-dangling intra-doc link in replay_double_and_add. Behavior-preserving: ecsm lib 15/15, clippy clean, production builds with no test deps. * Fix leftover unwrap and pub mod in ecsm/tests --------- Co-authored-by: jotabulacios <jbulacios@fi.uba.ar>
* refactor(math): move test-only fft helpers (get_powers_of_primitive_root, compose_fft) into the test tree * refactor(math): import IsSubFieldOf instead of fully-qualifying it in compose_fft test --------- Co-authored-by: MauroFab <maurotoscano2@gmail.com>
#689) * refactor(prover): move test-only Traces constructors + trim_zero_rows into src/tests/ Moves three `#[cfg(test)] pub fn` constructors (`from_logs_trimmed`, `from_logs_minimal`, `from_elf_and_logs_minimal`) and `trim_zero_rows` out of production source files (`trace_builder.rs`, `bitwise.rs`) into a new `prover/src/tests/trace_test_helpers.rs` module. All ~70 call sites are unchanged because the constructors live in an inherent `impl Traces` block in the same crate. No public API changes. * style(prover): cargo fmt * add missing #[cfg(test)] in trace_test_helpers * docs(prover): restore loud UNSOUND FOR PRODUCTION warning on test-only trace helpers --------- Co-authored-by: jotabulacios <jbulacios@fi.uba.ar> Co-authored-by: MauroFab <maurotoscano2@gmail.com> Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
* Inject LambdaVM crypto into the ethrex guest * Address review findings on ethrex-crypto provider - Add host-runnable end-to-end secp256k1_ecrecover known-answer tests (valid constructed ECDSA signatures + zero-r/zero-s negatives), covering the recovery wiring through the software fallback path. - Factor the keccak sponge behind an injected permutation (keccak256_with_permute) so it is host-testable against ethrex's keccak_hash across the rate/padding edge sizes. - Replace deprecated FieldBytes::from_slice with non-deprecated conversions (removes 3 build warnings). - Drop dangling "the plan" / "(Phase 1)" comment references in Cargo.toml. - Soften the point_from_xy comment: the on-curve check is a backstop, not a correctness guarantee. - Fix ecsm_oracle doc: arbitrary base point (not generator), real local names, uppercase N for the curve order. * Move ethrex-crypto tests into src/tests/ modules Match the repo test-layout convention (prover/executor src/tests/): split the inline `mod tests` in lib.rs into per-area files under src/tests/ declared from src/tests/mod.rs. - src/tests/ecsm_tests.rs — x-only lincomb2 reconstruction + fallbacks - src/tests/ecrecover_tests.rs — full ecsm_ecrecover known-answer/negatives - src/tests/keccak_tests.rs — keccak sponge vs reference keccak_hash * Add edge-case tests and fix misleading comments * route ecrecover address hash through precompile --------- Co-authored-by: MauroFab <maurotoscano2@gmail.com>
* infra: add moonmath provider for ai reviews * fix: apply code review comments
…ep (#709) Replace the PR benchmark's headline program (fib_iterative_8M) with the ethrex guest proving a 20-transfer block, and the memory-growth sweep (fib 1M..8M) with an ethrex transfer-count sweep (4/8/12/16/20). Transfers use N distinct, genesis-funded senders -> N distinct recipients ("distinct" mode), so the state-trie witness reflects a realistic block rather than repeated same-account transfers. - tooling/ethrex-fixtures: add optional `mode` arg (same|recipients|distinct). distinct injects deterministic synthetic senders into the genesis allocation and uses a per-index tip so block ordering (and output bytes) are reproducible. - benchmark-pr.yml: build the ethrex ELF + generate fixtures in-job (gitignored, not committed); prove with --private-input. Growth runs at default parallelism, 1 sample/point (run-to-run heap variance ~0). /bench-growth no longer forces k=1. - executor/.gitignore: ignore the generated bench fixtures.
…711) * refactor(make): proper per-file dep tracking for ASM programs Replace the shell for-loop in compile-programs-asm with a pattern rule so make tracks each .s → .elf pair individually and only recompiles files whose source changed. Remove the *-no-compile targets (test-asm-no-compile, test-rust-no-compile, test-no-compile) that existed solely to skip the unconditional loop rebuild; test-asm, test-rust, and test-executor now inline their cargo commands after their compile prerequisites. * refactor(make): use order-only dir prereqs for Rust/Bench pattern rules (#713) Move `mkdir -p` out of the Rust and Bench recipe bodies and into dedicated directory targets with order-only prerequisites, matching the pattern already used for ASM artifacts in this branch. --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
…h-abba tiebreaker (#712) * ci(bench): tune cheap tier (baseline 5, cap runs at 5) + escalation hint The single-session cached comparison can't beat the ~1% cross-session drift wall, so pushing either side past 5 runs buys little resolution. Set BENCH_RUNS_BASELINE=5 and clamp /bench N to 1-5 (the per-PR default stays 3 for fast feedback). When a PR shows a small time speedup (<1.5%) that the cheap CI can't confirm, the comment now suggests running /bench-abba. Also exclude /bench-abba from the regular bench trigger so it doesn't double-fire. * ci(bench): add /bench-abba manual ABBA tiebreaker job New issue_comment job (manual-only: a `/bench-abba` comment on a PR from a repo member; never auto-triggers) that runs the drift-free interleaved A/B/B/A paired benchmark and posts a paired-t CI + exact Wilcoxon test as a PR comment. It occupies the single self-hosted bench server for ~30-40 min, hence manual-only. Optional pair count via `/bench-abba N` (default 20; ~20 resolves 1%, 32 for 0.6%). Adds scripts/bench_abba.sh, which the job invokes to build both binaries (isolated worktree) and run the pairs. * ci(bench): address AI review — SHA-aware cache, fork PRs, N clamp, diagnostics Confirmed findings from the multi-model review: - critical/high: SHA-aware binary cache — rebuild when cli_{A,B}.sha don't match the requested SHAs (was existence-only, so a persistent /tmp on the self-hosted runner could silently benchmark a previous PR's binaries). - high: fork-PR head resolution — workflow now resolves headRefOid + fetches pull/N/head and passes the SHA (origin/<branch> doesn't exist for forks). - high: clamp /bench-abba N to [2,40] in the workflow (was unbounded -> DoS). - high: build output -> per-binary log, surfaced on failure (was >/dev/null). - high: prove runs capture stderr (2>&1) so prover failures are diagnosable. - medium: add timeout-minutes: 120 so a hang can't strand the bench runner. - medium: louder warning on git fetch failure. - low: REF_A is now required (dropped the hardcoded PR #696 default). - low: fail fast if python3 is missing (before the ~30-min build). Deliberately kept: shared cargo target across the two worktree builds (incremental 2nd build; cargo recompiles on source change, REBUILD=1 covers dep changes).
* add row major batched lde fft primitives * Make LDETraceTable row-major * Wire prover to row-major batched LDE * read trace row major * Move the batched-FFT and row-major-LDE unit tests into corresponding file * fix disk-spill EmptyCommitment in row-major LDE * Parallelize trace build and speed up op-dedup bookkeeping * Skip the identity multiply by alpha_powers[0] in LogUp fingerprints * Remove dead FFT module and gate legacy twiddles * Harden parallel row-major bit-reverse permute * Guard columns_to_row_major; clarify hasher doc * Deduplicate commit_rows_bit_reversed and bit_reverse_vec * Fix bit-reverse memory savings comment * use default hasher for op dedup maps * Use std HashMap directly for op-dedup maps --------- Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com> Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com>
) * perf(stark): skip fixed 0/1 muls in LogUp fingerprint accumulation In the fingerprint hot loop (prover aux-build + constraint-eval + verifier): - Bus-id term: alpha_powers[0] = alpha^0 = 1, so embed the bus id into the extension field directly instead of multiplying by 1 (drops one F*E mul per interaction per row, hoisted out of the row loop on the aux path). - Fixed-zero bus elements (the ~235 constant(0) used for bus-width padding) contribute nothing: skip the F*E multiply + accumulate entirely. Variable elements that happen to be zero on a row also benefit. Value-identical (field addition is exactly associative): stark lib 128/128 (default + parallel), prover bus/logup tests pass, clippy clean. Net effect on prove time is what we want to measure on the 32-core bench. * docs(stark): align fingerprint comments with the α⁰=1 optimization - compute_fingerprint_from_step: drop the vestigial *α^0 from the doc formula so it mirrors the code (and matches docs/cryptography/lookup.md and spec/logup.typ). - accumulate_fingerprint{,_from_step}: the zero-skip also covers variable elements that are zero on a row, not just the constant(0) padding — reword the inline comments to say so. --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com>
…computed twiddles) (#700) * perf(stark): fuse composition half-extension onto coset_lde_full decompose_and_extend_d2's extend_half_to_lde did iFFT(g²) → coefficient Polynomial → evaluate_polynomial_on_lde_domain(g) as two separate FFTs with an intermediate coefficient allocation per half. Replace with a single fused coset_lde_full: iFFT(n) → coset reshift g²→g → forward FFT(2n=lde_size). The weights (g⁻ʲ/n, folding the 1/n iFFT normalization and the net g²→g shift) and the inverse twiddles (size lde_size/2) are precomputed once per domain in LdeTwiddles (the forward FFT reuses the existing fwd twiddles), and threaded through prove_rounds_2_to_4 → round_2 → decompose_and_extend_d2 — no per-call recomputation. This path is now production (degree-3 tables use the 2-part decompose_and_extend_d2 after #699). Byte-identical: test_decompose_and_extend_d2_matches_original (decompose output == original break_in_parts path), a new formula test, stark 130/130, real VM proof (fib_iterative_1200k) prove+verify OK, clippy + fmt clean. * fix(stark): drop clone_on_copy in composition extend test (clippy -D warnings) * fix(stark): keep composition LDE twiddles in release builds * fix(stark): lazy composition LDE twiddle cache --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com>
* ci: add gpu benchmarks * add retries * ci: use ABBA method to run the benchmark * ci: use 64gb ram * ci: remove datacenter flag * fix: units for RAM * fix: min driver and ssh key * fix: rebuild binaries * fix: use correct sh * fix: use 64gb ram * fix: use expensive machine with $1 cap * fix: remove temporary code * fix: apply code review * test: run on push * fix: cuda * remove test setup * ci(bench-gpu): harden teardown, cap pairs at 32, fix CUDA comment (#736) Review follow-ups on the GPU benchmark workflow: - Teardown: fall back to destroying by the unique RUN_LABEL when no instance id was recorded. The id file is written only after `create` succeeds and its JSON parses, so a box created in that window (concurrency cancel, or a parse failure) could otherwise leak and bill indefinitely. - Cap pairs at 32 (was 40) and round odd requests up to even (the AB/BA design wants even N); raise the job timeout to 210 min so a worst-case 32-pair run (64 proves + slow provisioning + dual CUDA build) fits without timing out after the expensive build. - Fix the CUDARC_PIN comment: the boxes are ~CUDA 12.8 (matching cuda-12080 and the cuda_max_good>=12.8 offer floor), not 13.0; tie it to the MIN_DRIVER guard as the opposite end of the same compatibility window. - Log only the needed fields of create.json instead of the full --raw response, so an unexpected sensitive field can't land in the run log. - Validate the workflow_dispatch branch name before it is interpolated into the remote `bash -lc` command. - Move the run-summary write into an always() step so workflow_dispatch failures are visible in the Actions summary rather than only the raw step log. --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
* split execution into epochs * Add an initial-memory image * build a single epoch's traces * Add an is_final flag for halt * Make the HALT table optional * Add a register_init parameter to VmAirs::new for the REGISTER preprocessed commitment * reject a non-final epoch that contains the program-terminating instruction * Add local-to-global boundary and process epochs to emit the boundary set * Add local-to-global air table * Add cross-epoch local-to-global memory linkage * Add memory_bus_interactions to emit epoch init/fini tokens * Wire the local-to-global table as the epoch-local Memory-bus bookend * Add prove_and_verify_continuation * stream epochs one at a time and drop traces after proving * add bench_continuation * Add multi-pass array asm program (1 MiB footprint, ~20M steps) as a worst-case local-to-global memory stress benchmark * Add a count mode to bench_continuation that reports a program's cycle count by running the executor only, as a no-proving proxy for monolithic proving memory * l2g val to a single byte column * Thread private inputs so we can bench ethrex program * Use the static preprocessed bitwise commitment * Avoid redundant per-epoch work (skip page and carry the memory) * add global_memory for init-elf binding * add clasification into stack vs data/heap in bench_continuation * store memory in dense per-page arrays instead of per-cell HashMaps * update doc * Range-check the local-to-global continuation table columns * slim range-check since memw already does it * make fini_epoch constant, add MU selector for padding rows, add epoch ordering constraint * Gate the local-to-global MU selector on the GlobalMemory bus only and constrain it boolean, leaving the epoch-local Memory bus and the range/ordering checks on unconditional multiplicity so the cross-epoch init_epoch < fini_epoch ordering check can never be skipped via MU; padding rows stay harmless because they self-cancel on the Memory bus and send only valid range/ordering lookups. Also add a design doc describing the continuation local-to-global memory protocol, both MU-wiring designs, and the soundness reasoning. * Revert the local-to-global MU wiring to Design X (MU gates every L2G interaction, including the epoch-local Memory-bus bookend), because the Design Y variant that gated only the GlobalMemory bus is unsound: with the Memory bookend on unconditional multiplicity, a prover can set MU=0 on a non-first-touch row to orphan a touched epoch from the cross-epoch chain while its epoch proof still passes, and point the prover-controlled finalization at the truncation, silently dropping a real memory write. Gating the Memory bookend with MU forces MU=1 on every touched cell, which forces every touching epoch into the telescoping chain, making the chain complete and the finalization trustworthy. Update the design doc to record both designs, the chain-truncation attack, and the anchoring reasoning that makes Design X sound. * Bind cross-epoch register * Bind continuation epoch and global proofs to their statement in Fiat-Shamir (ELF, epoch label, epoch, count) * update md * carry the x254 commit index across epochs * Force continuation epoch size to a power of two * Use a power-of-two epoch size in tests * Add a test with a non-power-of-two epoch size * Split the integrated continuation prove+verify * CLI continuation flag * Remove dead-code allow and update doc * Thread ProofOptions through prove_continuation/verify_continuation * Seed the per-epoch touched-cell prediction from the carried register file instead of a fresh one * Validate each epoch's reg_fini length * Assert test_commit_across_epochs_verifies actually produces more than one epoch * Continuations cleanup: docs, comments, and regression tests (#714) Keep the follow-up scoped to non-performance cleanup while preserving the soundness regression coverage. - Add L2G/global-memory regression tests for MU selector behavior, chain truncation, l2g-root binding, and private-input continuations. - Fix stale continuation/global-memory docs and comments. - Replace bare x254 byte address literals with register_base_address(254). - Remove the unused DEFAULT_EPOCH_SIZE constant and document run_epochs as a test/bench helper. * Use log2 epoch size for continuation CLI (#717) Replace the continuation CLI's raw --epoch-size / --num-epochs controls with --epoch-size-log2. The CLI now computes an exact power-of-two epoch size directly, defaults to 2^20, rejects tiny log2 values below 18, and no longer runs a cycle-count pre-pass to split into a target epoch count. Update the continuation design doc and help text with the ethrex 10-transfer memory sweep as guidance. * Delete init_ts column and drop ts from GlobalMemory bus * Replace the always-zero global_memory init_epoch column with a verifier-fixed GENESIS_EPOCH constant * Represent init state with dense representation instead of intermediate HashMaps * Avoid duplicate L2G trace work in continuations (#719) * Reuse genesis page data for continuation global proof (#720) * Polish continuation verification and CLI (#728) * Make continuation API take epoch size log2 (#730) * Return continuation invariant errors instead of panicking (#731) * Simplify continuation L2G trace construction (#732) * Clean up continuation AIR setup (#733) * Reject continuations exceeding the IsB20 cross-epoch ordering range (#734) The cross-epoch ordering check proves `init_epoch < fini_epoch` via an IsB20 (20-bit) lookup on `fini_epoch - 1 - init_epoch`, so a run can have at most 2^20 epochs. Beyond that the IsB20 bus cannot balance and no honest proof exists. Previously this was guarded only by a debug_assert in the prover's bitwise emission, so a release build would build an unprovable trace and fail cryptically — reachable via the library API with a small epoch size (the CLI's min epoch size keeps it out of reach there). Add a hard check in `prove_continuation`'s epoch loop returning `Error::InvalidContinuationEpochSize` with a clear message once the epoch count would exceed the range. This is a prover-side guard only: the verifier already rejects any such proof (the IsB20 table is preprocessed and the ordering sender is rebuilt verifier-side from a positional epoch label), so soundness is unchanged — it just turns a confusing failure into a clean error. Introduce `local_to_global::MAX_EPOCHS` as the single source of truth, used by both the new check and the existing debug_assert (replacing the `1 << 20` literal). * add doc and debug_assert --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
* add row major batched lde fft primitives * Make LDETraceTable row-major * Wire prover to row-major batched LDE * read trace row major * Move the batched-FFT and row-major-LDE unit tests into corresponding file * fix disk-spill EmptyCommitment in row-major LDE * Parallelize trace build and speed up op-dedup bookkeeping * Skip the identity multiply by alpha_powers[0] in LogUp fingerprints * Remove dead FFT module and gate legacy twiddles * Harden parallel row-major bit-reverse permute * Guard columns_to_row_major; clarify hasher doc * Deduplicate commit_rows_bit_reversed and bit_reverse_vec * add gpu tests * Add GPU/CPU Merkle root parity test * Add GPU/CPU Merkle root parity tests for base and ext3 aux trace * Add GPU/CPU barycentric OOD parity tests * Fix ext3 pre-strided layout in barycentric parity test * Fix instruments double-billing GPU fused pipeline in R1 * Add test verifying GPU and CPU proofs both pass verification * Clean up verbose comments in parity tests * GPU R1 GPU: eliminate extract_columns + columns_to_row_major via on-device transpose * Revert "GPU R1 GPU: eliminate extract_columns + columns_to_row_major via on-device transpose" This reverts commit 38f5600. * GPU R1: row-major NTT kernel — no transpose, coalesced column access * Fix GPU R1 row-major: transpose buf to col-major for device handle * Fix keccak row-major launch config: use 128-thread block, not 1024 * GPU R1 aux: row-major ext3 NTT reusing base-field kernels with m*3 * Clean up GPU row-major LDE: extract transpose helper, fix zero-pad alloc, trim stale comments * Clean up GPU row-major LDE: extract helper, fix alloc, trim comments * Fix gpu_lde_threshold OnceLock: re-read env var in test builds * Fix cross-stream race: synchronize after transpose before returning handle * Add parity tests for new row-major GPU pipeline * Remove dead batched-keep GPU LDE functions * fix lint * Add debug_assert for Fp3 Vec::from_raw_parts invariant * Revert unrelated FxHashMap op-dedup change (out of scope for #715) The FxHasher/FxHashMap op-dedup micro-optimization is unrelated to the row-major GPU LDE rework and was only applied to 4 of 6 dedup tables. Revert the table maps to std HashMap and drop the hasher; it can land as its own focused PR. * Remove redundant gpu_and_cpu_proofs_both_verify test The GPU full path is covered by the normal prove/verify suite built with --features cuda (plus gpu_path_fires_end_to_end), the CPU path by the non-cuda suite, and GPU/CPU equivalence by the merkle/barycentric parity tests. Its force-CPU leg also never ran on CPU: gpu_lde_threshold() only re-read the env var under cfg(test), but from the prover integration crate stark compiles without cfg(test), so the OnceLock cached the first value. Simplify gpu_lde_threshold() to a single cached impl now that the per-call re-read has no consumer. * Fix stale docs and remove dead code keccak.cu: move keccak256_leaves_base_row_major out of keccak_merkle_level's doc block so the child-pair->parent doc rejoins its kernel. prover.rs: delete columns_to_row_major, which has no callers after the row-major GPU path stopped materializing GPU-expanded columns. * Consolidate row-major LDE pipeline; guard keccak num_rows Extract coset_lde_row_major_inner shared by the base and ext3 _keep entry points (they differed only by m vs m*3 and the handle type), removing ~110 lines of drift-prone duplication. Add debug_assert!(num_rows >= 2) to launch_keccak_base_row_major: the kernel shifts by (64 - log_num_rows), UB at num_rows==1, matching the guard in launch_keccak_base. * Fix stale R2 composition-LDE assertion in gpu_path_fires_end_to_end The assert checked gpu_parts_lde_calls() > 0 with a comment claiming branch/shift tables are degree-3 — both false: fib_iterative_1M tables all have number_of_parts <= 2, and the common degree-2 case fires the fused two-halves path (gpu_extend_halves_calls), counted separately from the parts>2 path (gpu_parts_lde_calls) since #700. Assert on the sum so either composition-LDE path satisfies it. Validated on RTX 5090 / CUDA 13.1: make test-math-cuda 78/78, make test-cuda-integration green, proof verifies. --------- Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com> Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com>
* refactor(stark): extract Merkle commitment into a commitment module Unifies the two near-identical bit-reversed leaf hashers (per-row + per-row-pair) into one keccak_leaves_bit_reversed_grouped(columns, rows_per_leaf), and the two near-identical commit fns (commit_columns_bit_reversed + commit_composition_polynomial) into commit_bit_reversed(columns, rows_per_leaf), in a new crypto/stark/src/commitment.rs. Removes them from the IsStarkProver trait (they used no self). prover.rs re-exports the named leaf hashers for the math-cuda GPU parity test. Byte-identical (stark 128/128). * perf(stark): row-pair the trace commitment (one Merkle path per query) Every FRI query opens a value and its symmetric counterpart (LDE positions 2*iota, 2*iota+1). The composition-poly commitment already grouped that pair into one leaf; the trace (main/aux/precomputed) committed one row per leaf and opened TWO leaves (proof + proof_sym) per query. Commit the trace with rows_per_leaf=2 too, so each query opens ONE leaf/path; drop the now-redundant proof_sym from PolynomialOpenings (also shrinks the composition opening, which stored the path twice). verify_opening_pair now reconstructs the paired leaf and verifies once (mirrors verify_composition_poly_opening); the dead verify_opening is removed. Halves trace Merkle authentication-path data per query (smaller proofs + less verifier hashing). stark 128/128 (prove+verify). NOTE: proof FORMAT change (not byte-identical). cuda follow-up: the GPU trace leaf+tree builders (gpu_lde::try_expand_leaf_and_tree_batched_keep/_ext3_keep + math-cuda kernels) still build 1-row leaves and must switch to the row-pair pattern (the GPU composition builder already pairs) or cuda proofs will fail verification. * refactor(stark): prover cleanup — par helpers, ROWS_PER_LEAF, error propagation, doc/log fixes * docs(stark): fix commitment.rs leaf-layout docs after trace pairing The trace commitment now uses the row-pair leaf layout (ROWS_PER_LEAF=2), same as composition; rows_per_leaf=1 is only kept for the GPU parity test. Update the module/const/wrapper docs that still described the pre-pairing per-row trace. * refactor(prover): dedup commit pipeline (commit_plain + spill_tree) (B) Extract two helpers on IsStarkProver, collapsing the near-duplicate main-trace and aux-trace commit code: - spill_tree<C>: the identical-except-label disk-spill block, shared by the main / preprocessed-split / aux commit sites (4 call sites). - commit_plain<C>: commit_bit_reversed + spill_tree + TableCommit::plain, shared by the main-trace (non-preprocessed None arm) and aux-trace plain-commit paths. Proof output is byte-identical (stark 128/128, +disk-spill 133/133). The only behavioral delta is in the instruments profiling feature: the aux commit's timing bucket now includes its (tiny) disk spill, matching what the main path already measured. clippy clean on default / disk-spill / instruments; builds on the combined feature set. * fix(prover): row-pair the preprocessed-table commitments (CI fix) The trace Merkle commitment moved to a row-pair leaf layout (ROWS_PER_LEAF=2), but the 5 preprocessed-table commitment computers (bitwise/keccak_rc/page/decode/register) still built a 1-row-per-leaf tree manually, so the prover's row-pair precomputed root no longer matched the computed/hardcoded one -> PrecomputedCommitmentMismatch at prove time. - Route all 5 compute_*commitment fns through the shared stark::commitment::commit_bit_reversed(.., ROWS_PER_LEAF), dropping the manual bit-reverse + columns2rows + 1-row BatchedMerkleTree::build. - Regenerate the hardcoded bitwise/keccak_rc/zero_page static commitments for the row-pair layout (via compute_static_commitments). - cargo fmt (prover.rs + touched tables). Verified: static_commitments drift tests 5/5, stark 128/128, clippy + fmt clean, no PrecomputedCommitmentMismatch. Full ELF prove/verify runs in CI (guest artifacts absent locally). * test(prover): regenerate SUB_DECODE_COMMITMENT_BLOWUP_2 for row-pair layout The compile-time decode-commitment const for sub.elf shifted with the row-pair preprocessed commitment; regenerated via commitment_from_elf (the print_decode_commitment_for_sub regen path). * test(prover): TEMP print actual decode commitment to regenerate const from CI * test(prover): set SUB_DECODE_COMMITMENT_BLOWUP_2 to CI-computed row-pair value Regenerated from CI's sub.elf (local riscv toolchain unavailable); removed the temporary print instrumentation. * mplement changes in GPU * refactor * Fix CUDA LDE clippy lint * fix(cuda): align review cleanup with row-pair commits (#723) * fix(stark): silence dead_code on par_for_each_mut (debug-checks-only after merge) * style(stark): cargo fmt after merge resolution * test(cuda): focused GPU row-pair commitment prove+verify test * fix(cuda): drop stale R2 parts-LDE asserts (#700 fused path), silence GPU column-LDE dead_code The R2 parts-LDE / comp-poly-tree GPU dispatches no longer fire since #699/#700 route degree-3 tables through the 2-part fused coset_lde_full path (no AIR has number_of_parts > 2). try_expand_columns_batched* are debug-checks-only after the #650 row-major LDE became production. * review(stark): address PR #735 review — coverage, dead code, cleanups (#740) Follow-up to the row-pair commitment PR. Excludes the intentional proof-format break (proof_sym removal / leaf-count change), which is by design. Test coverage (in their own files under crypto/stark/src/tests/, per the crate's test structure): - tests/commitment_tests.rs: direct unit tests pinning the row-pair leaf layout (R=1 and R=2) against an independent reference, wrapper agreement, commit-root consistency, and empty-input short-circuit. Previously the leaf layout was only covered transitively via full prove->verify, and GPU parity tests compared against an inline reimpl rather than this module. - tests/row_pair_opening_tests.rs: two negative tests for the row-pair verify_opening_pair — a tampered symmetric trace evaluation and a corrupted Merkle authentication path must both be rejected. Removing proof_sym deleted the old "symmetric opening mismatch" rejection class; these restore it (an impl ignoring evaluations_sym / the auth path would otherwise pass every existing test). Reuses the now pub(crate) make_valid_simple_proof helper. - cuda_path_integration.rs: restore assert!(gpu_comp_poly_tree_calls() > 0). try_build_comp_poly_tree_gpu is dispatched unconditionally (round 2, after the parts-count branch), so it fires for the common number_of_parts == 2 (degree-3) case — it was NOT obsolete. Keep the genuinely-dead parts-LDE assertion dropped. Cleanups: - Delete dead fn commit_plain (zero callers; main/aux commit inline commit_rows_bit_reversed + spill_tree, which are row-major and incompatible with its column-major signature — the dedup never landed). - Delete orphaned pub fn columns2rows (all callers removed by #735). - Add ProvingError::Fft and map FFTError to it instead of WrongParameter (internal FFT failure is not a caller-supplied-parameter error). - Fix stale profiler label commit_composition_poly -> commit_bit_reversed. - Drop the rot-prone "// = 2" comment on the local ROWS_PER_LEAF alias. * review(stark): tidy test layout + remove AGENTS.md (follow-up to #740) (#744) Two items that landed too late for #740: - Move the shared make_valid_simple_proof helper out of small_trace_tests.rs into tests/trace_test_helpers.rs, where the crate keeps shared test helpers (matching how prover_tests sources get_trace_evaluations). row_pair_opening_tests.rs and small_trace_tests.rs now both import it from there instead of one test file reaching sideways into another. - Delete AGENTS.md (added by #735). --------- Co-authored-by: Joaquin Carletti <joaquin.carletti@lambdaclass.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com> Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
…120 HWSL sends/row (#889) Replace KECCAK_RND's 120 HWSL bus sends per row (θ rotate-by-1: 20, ρ shifts: 100) with degree-2 linear identities over the same committed cells: μ · (in · 2^rnc − right · 2^16 − left) = 0. The existing IS_BYTE and IS_BIT checks make the split unique — given left, right ∈ [0, 2^16), the pair is the Euclidean quotient/remainder of in · 2^rnc ÷ 2^16, and all values stay < 2^32 ≪ p. Sends/row 1151 → 1031 ⇒ −60 aux extension columns (−180 committed base cells/row, ~4.3k/permutation), zero new columns. Constraints 20 → 140, all degree ≤ 3; max_degree() unchanged. Measured −6.8% median prover time on a pure-keccak guest (see the PR for the full A/B). Matches the chip spec as updated by the research team on spec/main (d397668, #873).
…nuations (#875) * new opt * fix(gpu): harden round-2 residency paths after review Grid-stride the fused row-major NTT past gridDim.y (lde >= 2^24 silently fell back to CPU), assert the device-only contract in the R2 composition commit and preprocessed opening fallbacks, validate htod_via bounds, retain FRI device evals only under device-only, and move the inverse fault-injection hook so every batch-inverse entry is covered. * fix(gpu): address round-2 review — guarded zero-inverse, retargeted fault hook, merkle root-only * perf(gpu): stage htod_via in fixed 64MB chunks to bound pinned footprint * style: rustfmt htod_via chunk-size expression * fix(gpu): gate R2 comp-tree host fallback on the parts, not host_trace_empty * chore(gpu): review follow-ups — gather bounds, release canaries, live zero-total guard - gather_ext3_at asserts positions against the evals buffer host-side (same guard as gather_merkle_paths_dev). - The device-gather cross-checks keep query 0 as a release canary instead of paying every query; debug still checks all of them. - The batch-inverse zero-total guard also compiles under test-faults, so the GPU fallback suite (which runs --release) actually exercises it. - New htod_via round-trip test covering the 64 MB chunk loop and its partial tail. * fix(gpu): drain htod_via on error; narrow the merkle-tail threshold (#892) * fix(gpu): drain htod_via on error, guard the R2 host-evaluator fallback Review follow-ups for the round-2 residency work, rebased onto e75bcbe — only the items that commit did not already cover. htod_via error path. Once a chunk's DMA is in flight, `record_event` / `sync_event` returning `Err` drops the staging `MutexGuard` with the device still reading the pinned slab, so the next locker's `ensure_capacity` can `cuMemFreeHost` it mid-copy. `async_dtoh_via` already guards this exact hazard and the file ships a `DrainOnErr` helper for it; `htod_via` was the one site not using it. R2 host-evaluator fallback. If the device decompose and the `H` download both fail under device-only, control reaches the host evaluator, which reads the intentionally-empty trace and panics with a bare out-of-bounds. Assert the device-only contract instead, matching the other fallback arms. Coverage. `batch_inverse_ext3_dev`'s `n == 1` branch is never exercised — `batch_inverse_n1` goes through the host-only short circuit in `batch_inverse_ext3`, as its own comment says. Add a direct device test. Docs. The preprocessed split-tree comment still claimed both trees come back as full host trees (the multiplicity tree is root-only + device resident), and `FriCommitState`'s doc claimed its input is always Arc-shared with a retained `gpu_evals` (only true on the device-only path). * perf(gpu): set the merkle-tail threshold to the block width TAIL_MAX_PAIRS = 2048 overshoots. The tail grid-strides a single 128-thread block on one SM, so a level of k pairs is k/128 SEQUENTIAL keccak-f1600s where the per-level launches it replaces spread them over k/128 parallel blocks. At 2048 the first four levels alone are 16+8+4+2 = 30 serial permutations against 4 parallel waves — order +100 us per large tree to save 4 launches worth order 10 us, and it sits on the critical path because the caller's 32-byte root memcpy_dtoh host-blocks on everything queued before it. At the block width the entry level is exactly one permutation per thread, so the tail still collapses the top levels into one launch but adds no serialization at all. --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
…#877) * new opt * fix(gpu): harden round-2 residency paths after review Grid-stride the fused row-major NTT past gridDim.y (lde >= 2^24 silently fell back to CPU), assert the device-only contract in the R2 composition commit and preprocessed opening fallbacks, validate htod_via bounds, retain FRI device evals only under device-only, and move the inverse fault-injection hook so every batch-inverse entry is covered. * fix(gpu): address round-2 review — guarded zero-inverse, retargeted fault hook, merkle root-only * perf(gpu): stage htod_via in fixed 64MB chunks to bound pinned footprint * style: rustfmt htod_via chunk-size expression * fix(gpu): gate R2 comp-tree host fallback on the parts, not host_trace_empty * chore(gpu): review follow-ups — gather bounds, release canaries, live zero-total guard - gather_ext3_at asserts positions against the evals buffer host-side (same guard as gather_merkle_paths_dev). - The device-gather cross-checks keep query 0 as a release canary instead of paying every query; debug still checks all of them. - The batch-inverse zero-total guard also compiles under test-faults, so the GPU fallback suite (which runs --release) actually exercises it. - New htod_via round-trip test covering the 64 MB chunk loop and its partial tail. * fix(gpu): drain htod_via on error; narrow the merkle-tail threshold (#892) * fix(gpu): drain htod_via on error, guard the R2 host-evaluator fallback Review follow-ups for the round-2 residency work, rebased onto e75bcbe — only the items that commit did not already cover. htod_via error path. Once a chunk's DMA is in flight, `record_event` / `sync_event` returning `Err` drops the staging `MutexGuard` with the device still reading the pinned slab, so the next locker's `ensure_capacity` can `cuMemFreeHost` it mid-copy. `async_dtoh_via` already guards this exact hazard and the file ships a `DrainOnErr` helper for it; `htod_via` was the one site not using it. R2 host-evaluator fallback. If the device decompose and the `H` download both fail under device-only, control reaches the host evaluator, which reads the intentionally-empty trace and panics with a bare out-of-bounds. Assert the device-only contract instead, matching the other fallback arms. Coverage. `batch_inverse_ext3_dev`'s `n == 1` branch is never exercised — `batch_inverse_n1` goes through the host-only short circuit in `batch_inverse_ext3`, as its own comment says. Add a direct device test. Docs. The preprocessed split-tree comment still claimed both trees come back as full host trees (the multiplicity tree is root-only + device resident), and `FriCommitState`'s doc claimed its input is always Arc-shared with a retained `gpu_evals` (only true on the device-only path). * perf(gpu): set the merkle-tail threshold to the block width TAIL_MAX_PAIRS = 2048 overshoots. The tail grid-strides a single 128-thread block on one SM, so a level of k pairs is k/128 SEQUENTIAL keccak-f1600s where the per-level launches it replaces spread them over k/128 parallel blocks. At 2048 the first four levels alone are 16+8+4+2 = 30 serial permutations against 4 parallel waves — order +100 us per large tree to save 4 launches worth order 10 us, and it sits on the critical path because the caller's 32-byte root memcpy_dtoh host-blocks on everything queued before it. At the block width the entry level is exactly one permutation per thread, so the tail still collapses the top levels into one launch but adds no serialization at all. * perf(prover): replace table chunks with a VRAM-admitted per-table scheduler Fiat-Shamir only requires the main roots absorbed in index order before the shared challenges; past that fork every table's chain is independent. Phase A now runs all main commits under a byte-budget admission gate (no chunk barriers), and aux build, aux commit and rounds 2-4 run fused as one task per table, heaviest first — while a big table works through a host-bound stretch, the other tables' GPU stages fill the device. GPU builds default TABLE_PARALLELISM to 2/3 of the cores (swept flat at 10 on a 16-core RTX 5090). ethrex 10tx continuations on RTX 5090: 10.64s -> 8.54s (-19.7%, 8 ABBA pairs). * fix(prover): repair the instruments span tree and timing report under the per-table scheduler (#893) * fix(instruments): nest per-table spans under their real parent The per-table scheduler moved `r1_aux_build`, `r1_aux_commit` and `rounds_2to4` inside closures that run on `std::thread::scope` worker threads. `SPAN_DEPTH` is thread-local and a fresh OS thread starts at 0, so all three were stamped `depth = 0` and recorded as root siblings of `prove_total` instead of children of `proving`. That happens even at k = 1. Downstream, `scripts/profiling/phase_table.py` reconstructs the tree with `del stack[d:]`, so a depth-0 span empties the ancestor stack and `prove_total` stops being an ancestor of anything — the "% of total" column documented in `scripts/profiling/README.md` becomes meaningless. `run_admitted` now reads the spawning thread's depth and seeds each driver with it via new `instruments::current_depth` / `enter_depth`. Both call sites are `#[cfg(feature = "instruments")]`, so non-instrumented builds are byte-identical. Also correct the module contract doc: per-table spans genuinely do overlap now — that is inherent to running one driver per in-flight table, not a bug to code around. Only the top-level phase spans remain a strict latency breakdown. * fix(prover): report aux build/commit where they actually accrue `aux_build_elapsed` / `aux_commit_elapsed` were hardcoded to `Duration::ZERO`, but `prover/src/instruments.rs` still computed `round1 = main_commits + aux_build + aux_commit` and still printed the "Aux trace build" / "Aux trace commit" rows. Since `accum_r1_aux` keeps firing, the report showed nonzero LogUp and Aux-LDE/Merkle children under zero parents, and all the aux time silently landed in "Rounds 2-4". Time both stages inside the fused chain and sum them across drivers (`instruments::accum_aux_phases` / `take_aux_phases`), then restructure the report to match what the scheduler actually does: - "Round 1 (main trace commits)" is now exactly the main commits — the last phase-wide barrier, since the main roots must all be in the transcript before the shared LogUp challenges are sampled. - Aux build, aux commit and rounds 2-4 sit under one wall-clock parent, "Rounds 2-4 (aux build+commit fused)", with the aux rows marked as summed across concurrent drivers — they may exceed that wall, the same convention the existing accum_* sub-rows already use. No zero parents over nonzero children remain. Verified on fib_iterative_1M: Round 1 1.68s, Rounds 2-4 6.89s wall, aux build 2.91s and aux commit 3.38s summed over 5 drivers. * fix(bench): drop the heap guards whose snapshots no longer exist The scheduler removed `instruments::snap("After aux build")` and `snap("After aux commit")`. `bench_prover_scaling.sh` still parsed them, printed them and ran heap-growth regressions on them, so two regression guards were comparing nothing and dropping out without complaint. Re-adding a snapshot inside a per-table task would be meaningless — with k tables in flight there is no single moment at which aux build or aux commit has finished — so remove the two rows and their `regress` calls, with a NOTE recording why and pointing at the guards that still cover the fused region ("After main commits" and "Peak heap"). Also repoint the timing regexes at the labels the report actually prints. `Main expand_columns_to_lde` / `Aux expand_columns_to_lde` and `Main commit (Merkle)` / `Aux commit (Merkle)` had not matched since the labels gained their GPU/CPU suffixes, so t_main_lde, t_aux_lde, t_main_merkle and t_aux_merkle silently printed blank. All four populate again — checked by running the script's own awk over a real report. * docs(prover): refresh the comments the per-table scheduler invalidated Nothing functional. All of these described structure the scheduler removed: - `VramGate`'s rustdoc opened with the deleted `plan_table_chunks`'s doc comment ("Plan contiguous table chunks... Returns (start, end) half open ranges"), left behind and contiguous with `VramGate`'s own. - `Lde`'s doc claimed all N tables' LDE columns are live simultaneously. Only the main LDEs still are — the Round 1 main commit is a phase-wide barrier. Each aux LDE is produced and consumed inside one fused task, so at most k coexist. That is a memory improvement the PR made and did not claim; state the real, asymmetric bound. - A "Split into two passes for parallelism: Pass 1 ... Pass 2 ..." block sat two lines above the new comment saying the opposite. - `table_parallelism`'s doc still gave only `num_cores / 3`. Document both arms, that `TABLE_PARALLELISM` overrides both, and that without the `parallel` feature it is hardcoded to 1. - `run_debug_checks` said "called once after Phase C commits"; it now runs between two `run_admitted` passes. Document that, and the "each driver locks only its own index" contract its new `&[Mutex<AirTracePair>]` parameter relies on. - `auto_storage::peak_bytes` described phase D and a "worst possible chunk assignment". With `heaviest_first` the top-k is the set actually admitted first, not a worst case. Also document that `table_parallelism()` is not only the prover's k: `decide` feeds it into the RAM-vs-Disk choice, so the cuda arm's `cores * 2 / 3` doubles that transient term and makes `Disk` likelier. The direction is safe (it over-estimates) but was undocumented. - Remaining "Phase A/B/D" references, plus the "chunks of K" banner and the "Phase D's zip chain" handle comments. * test(prover): cover VramGate, run_admitted and heaviest_first These three had zero direct tests, and PR CI never exercises them concurrently: `ubuntu-latest` has 2-4 vCPU so `cores / 3` floors to k = 1, and `VramGate` is inert on non-cuda builds because `vram_budget = u64::MAX` makes `acquire`'s admit condition always true, so the condvar is never waited on. They are free functions over `&[u64]` with no field, AIR or GPU dependency, so a plain `#[cfg(test)] mod` pins them without a device: - `heaviest_first` returns a permutation of `0..n`, descending by estimate, with ties broken by index (stable sort — so the admission order does not vary run to run). - `run_admitted` fills every slot exactly once, including `order.len() == 0`, `workers > order.len()`, `workers == 1` and `workers == 0`. - `VramGate` admits an over-budget request alone rather than deadlocking, never lets concurrent admissions push `used` past the budget, and wakes waiters on permit drop. - A `u64::MAX` budget never blocks, including when the byte sum saturates. - `run_admitted` seeds its drivers' span depth, which guards the regression fixed earlier in this branch. Reads the depth directly rather than the global span timeline, which other tests in this binary also write to. Deterministic and fast: no sleeps as synchronization: channel rendezvous for ordering, and `recv_timeout` only as a failure deadline so a regression fails instead of hanging. * docs(gpu): record why scheduler drivers share pinned-staging slot 0 Per-driver slots were measured: repeated pinned allocation costs more than the shared mutex, whose transfers cross-table overlap already hides. * fix(instruments): restore the rounds 2-4 phase wall, make the prover timing report honest (#895) * fix(instruments): restore the rounds 2-4 phase span instead of plumbing depth Supersedes the approach in #893. Adversarial review showed the depth field was never the defect. `phase_table.py:121` takes its denominator from `max(s["wall_ns"] for _, s in pathed)` — the longest span, not the root of the ancestor stack — so depth-0 records never broke the "% of total" column, and `scripts/profiling/README.md:77` was accurate all along. `prover/src/continuation.rs` has also recorded spans from worker threads since long before this branch (:1146, :1205, :1299, :1328, :1415), with the comment at :1051-1053 saying so. Seeding worker depth was therefore work that bought nothing, and it would have left overlapping siblings looking like a clean tree — a subtler lie. Removed (`instruments::current_depth` / `enter_depth` / `DepthGuard` and the seeding in `run_admitted`). The real defect is label collision under summing. `phase_table.py:129` does `e["wall_ns"] += s["wall_ns"]`, so spans sharing a label are summed. On origin/main `rounds_2to4` was ONE span around the chunk loop (prover.rs:3503) and measured the phase; this branch made it one span per table, so the row became the sum of N concurrent tables — up to k times the real wall, able to exceed 100% — and no span measured the phase at all. `r1_aux_build` and `r1_aux_commit` were phase spans on main too (:3143, :3225). So: reopen `rounds_2to4` on the calling thread around the whole fused region, and rename the per-table spans `*_table` so a per-instance label can never be summed into a phase row. This also repairs `LAMBDA_VM_NSYS_CAPTURE_SPAN=rounds_2to4` (README.md:115), which with the label on the per-table span had N driver threads calling cuProfilerStart/Stop, the first to finish ending the capture. The report follows, and is compile-coupled to the same change. #893 added per-driver aux timers to fill the zeroed `aux_build` / `aux_commit` buckets; the fused stages have no wall-clock phase of their own any more, so reporting one invites exactly the misreading the label summing caused. Both timers and both `MultiProveTiming` fields are gone. The report now shows only the two phases that remain — "Round 1 (main trace commits)" and "Rounds 2-4 (aux build+commit fused in)" — with the aux CPU-time rows grouped under the fused phase behind headers stating they are summed over tables. That still fixes what #893 set out to fix: no row prints a fabricated 0.00s over live children, and "Round 1" no longer duplicates its own child. Verified on fib_iterative_1M: phase spans sum to their parent (r1_prepass 0.148 + r1_main_commit 2.493 + rounds_2to4 8.943 = 11.584 vs proving 11.585). * revert: trim the bench script back to the minimum #893 also repointed four timing regexes in `scripts/bench_prover_scaling.sh` that had gone stale earlier and independently of this branch. That is unrelated churn in a script with no Makefile target and no workflow referencing it, so it is reverted. What stays removed: the two dead heap rows and their `regress` calls (their `snap()` sources no longer exist and cannot be recreated with k tables in flight) and the two aux timing rows, which follow the report. The NOTE explaining why is kept. Nothing here was failing silently, contrary to the original review note: `regress` prints "(insufficient data)" for a missing key and `print_row` prints "-". * test: drop the scheduler unit tests * ci: force k > 1 on one prover shard so the scheduler runs concurrently Replaces the `VramGate` / `run_admitted` / `heaviest_first` unit tests added in #893 (removed in the previous commit). Every assertion they made was guaranteed by construction, already covered end to end, or unreachable from the call sites: `heaviest_first` is `(0..n).collect()` plus `sort_by_key`; a slot mixup in `run_admitted` is schedule independent, so it trips one of the three `.expect("run_admitted fills every slot")` sites or fails `multi_verify` on every PR today; `order.len() == 0` and `workers > order.len()` cannot happen, since `k` is `.max(1)`'d and `order` is always a full permutation. The one property with teeth — an over-budget table admitted alone — HANGS rather than fails if it regresses, which on an 8-10 minute shard burns to the job timeout unless wrapped in a watchdog. That was ~50 lines of permanent maintenance against approximately zero risk. The actual PR-time gap is that the scheduler never runs concurrently. `table_parallelism()` defaults to `(cores / 3).max(1)` and every job in this workflow is `runs-on: ubuntu-latest` with no larger-runner label, so PR CI proves with exactly one driver thread; `VramGate` is additionally inert on non-cuda builds, where `vram_budget` is `u64::MAX` and `acquire`'s condition always holds. `TABLE_PARALLELISM: 6` on shard 1 only is the smallest change that puts several real table closures in flight at once. The other three shards keep default-k coverage — the expression yields an empty string there, which fails to parse and falls back to the default. `prover/Cargo.toml:8` is `default = ["parallel"]`, so the env arm is the live one. Not a substitute for GPU coverage: `gpu-tests.yml` on merge_group rents a >=16-core RTX 5090, taking the cuda arm (`cores * 2 / 3`) with a finite VRAM budget, so both the concurrent and blocking paths already run before merge. This closes the PR-time gap only. * fix(bench): drop the row killed by the Round 1 relabel "Round 1 (main trace commits)" is lowercase, so `/Main trace commits/` stopped matching, and the row would have printed "-". It is also now redundant: with the aux stages fused out of round 1, `t_main_commits` and `t_round1` are the same number by construction. --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
…on verify paths (#855) verify_epoch and verify_global call multi_verify_views directly, without the marker prover/lib.rs's monolithic verify_proof_parts emits before its own multi_verify_views call. The recursion-block profile test buckets cycles by the latest marker observed, so on the continuation path "multi_verify setup (transcript replay phase A/B, per-table fork)" cycles were silently folded into whichever bucket was already active (airs_and_bus_balance for the first epoch, step4:openings carried over for later epochs), reporting the step at a flat 0.
…886) * perf(guest): read the private input zero-copy via ef_io::read_input get_private_input() to_vec()'s the whole memory-mapped input before rkyv deserializes it; read_input hands rkyv a slice straight into the input region instead. Same bytes, same private-input commitment. Measured vs origin/main (same fixtures, deterministic): transfers_20 8,732,213 -> 8,692,490 (-39,723) erc20_20 10,328,222 -> 10,278,822 (-49,400) mixed_20 9,817,444 -> 9,768,492 (-48,952) Verified: test_prove_ethrex_empty_block (prove+verify) passes. * fix(guest): take the zero-copy input via the safe get_private_input_slice (#898) The zero-copy read is the right call, but it hand-rolls what `syscalls::get_private_input_slice` already does: borrow the mapped private-input region in place and hand back `&'static [u8]`, no copy and no allocation. `get_private_input` is that same call plus a `to_vec()`, so dropping to the slice is the whole win without the pointer plumbing. Three things that buys: - No raw pointers in guest code. `syscalls.rs` deliberately keeps the region layout and its one `unsafe` block in a single place — that is why `get_private_input_slice` exists. Re-reading the length prefix in the guest duplicates layout knowledge that has to stay in step with the executor. - Restores the length-prefix clamp. `get_private_input_slice` bounds the prefix by `MAX_PRIVATE_INPUT_SIZE`; `ef_io::read_input` returns it raw. The executor rejects oversized inputs, so honest runs are identical — but a forged prefix built a slice reaching past the region instead of a bounded one. - Drops a dependency on unspecified behavior. `ef_io::read_input` documents `buf_ptr` as unspecified when `buf_size == 0`, and the previous code fed it to `from_raw_parts` regardless. Harmless in practice (the implementation always writes it, and ethrex input is never empty), but not a contract to lean on. `bench_vs/lambda/recursion` already reads its blob this way. --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
…st their sum (#909) * fix(verifier): pin each trace-opening column width to the AIR, not just their sum The verifier pinned only the SUM of a query opening's precomputed/main/aux column counts (against the AIR-pinned OOD width). Nothing pinned the split, and the Merkle leaf hash pins neither: hash_data_from_slices streams evaluations || evaluations_sym with no length prefix and no separator. Each of the three trees is transcript-bound at a different time, so both splits are exploitable: * precomputed<->main: a non-preprocessed AIR never absorbs the precomputed root, so columns declared 'precomputed' are bound by nothing. A prover can sample the round-2 challenges and then solve for them. * main<->aux: the aux root is absorbed after the shared LogUp challenges, so a column moved from main to aux is chosen after challenges it must precede. trace_opening_widths_well_formed pins all three widths, for both the regular and the symmetric slot, once per table before any opening is read. Co-Authored-By: diegokingston <dkingston@fi.uba.ar> * test(verifier): regression tests for the trace-opening column split Six end-to-end cases against a hostile prover that declares one column 'precomputed' for an AIR that is not preprocessed, plus direct tests of the guard on a RAP proof covering all three widths in both the regular and the symmetric slot. On stock main, three of these fail (the proof is accepted): the honest trace under a split declaration, the adaptively forged trace, and a demonstrably false statement. The other three pass on both and are the non-vacuity controls - in particular a genuinely preprocessed table, which has num_precomputed_columns() > 0, must still verify. The end-to-end cases need TEST_ONLY_SKIP_PRECOMPUTED_ROOT_ABSORB: a hostile prover does not absorb a root the verifier never reads, and without that the same proof is rejected for transcript divergence instead of for its split, which would prove nothing. Co-Authored-By: diegokingston <dkingston@fi.uba.ar> * style: cargo fmt + drop redundant clones flagged by clippy Co-Authored-By: diegokingston <dkingston@fi.uba.ar> * test(verifier): regression tests for the main<->aux opening split (LogUp break) Ports the aux-instance PoC into a permanent regression: a hostile AIR declaring layout (4, 2) against LogReadOnlyRAP's honest (5, 1) moves the multiplicity column into the auxiliary tree, which is transcript-bound only AFTER the shared LogUp challenges. The prover then solves that column against the sampled z/alpha, and the multiset equality the AIR exists to enforce degenerates into one scalar equation. On stock main both break tests are accepted - the structural mis-split and a false memory read (address 3 carrying two values) - the latter also over the rkyv wire through multi_verify_archived, the recursion-guest path. Unlike the precomputed instance this needs no prover change at all: both sides absorb main-root-then-aux-root either way. Three controls (corrupted aux opening, the same lie without the split, the split without the challenge solve) plus an honest LogReadOnlyRAP round trip pass on both, so the harness discriminates and the pin is not vacuous. Co-Authored-By: diegokingston <dkingston@fi.uba.ar> * docs(verifier): record the aux instance at verify_trace_openings and in the guard doc The aux arm authenticates against the aux root but constrains no width; say so, and point at the upstream pin. Same class of stale comment as the two this PR already corrects. Co-Authored-By: diegokingston <dkingston@fi.uba.ar> * test(verifier): drop the prover hook - both instances now pin hook-free The precomputed regression no longer needs the #[cfg(test)] absorb switch in prover.rs. Handing the prover and the verifier AIRs that disagree about num_precomputed_columns, while both absorb the same commitment constant, keeps the transcripts in sync - so the honest in-repo prover builds a proof that stock main accepts and this branch rejects. prover.rs is back to stock: the whole change is now verifier + tests. What the dropped end-to-end tests covered is kept: the 'a non-preprocessed AIR must declare zero precomputed columns' direction is pinned by the direct guard tests (its end-to-end form is masked by transcript divergence and proves nothing on its own), and the aux file demonstrates an executed false statement. Adds a tripwire (precheck_the_width_pin_is_compiled_in) plus attribution asserts in the break tests, so a rejection cannot be read as evidence unless it comes from the guard - the failure mode that made a sibling PoC look non-reproducing. Co-Authored-By: diegokingston <dkingston@fi.uba.ar> * docs(test): state precisely what the round-1 root check does and does not catch The precomputed-width test's comment implied real preprocessed tables are exploitable through this shape. They are not directly: an honest constant is a root over exactly num_precomputed_columns() columns, so a narrower tree hashes differently and round 1 rejects it. Say that, and say why the defence is incidental - nothing states the invariant, nothing checks it, and it is absent entirely for a non-preprocessed AIR. Co-Authored-By: diegokingston <dkingston@fi.uba.ar> * docs(verifier): trim the opening-width doc to the invariant The header carried the two exploit narratives in full, at ~33 lines for a ~40 line function -- 3x the sibling ood_blocks_well_formed. The mechanics belong in the tests that demonstrate them and in the PR; the header only needs the invariant, why an unpinned split is exploitable at all, and where to look. Co-Authored-By: diegokingston <dkingston@fi.uba.ar> --------- Co-authored-by: diegokingston <dkingston@fi.uba.ar>
…ory contents (two invariants, both with exploits) (#904) * fix(page): preprocess OFFSET on private-input pages A private-input PAGE (and its continuation analogue GLOBAL_MEMORY) skipped `with_preprocessed` entirely, so every column was prover-chosen main trace. PAGE carries `EmptyConstraints` and no constraint anywhere references `cols::OFFSET`, so nothing pinned it — and the Memory-bus address is `address_lo = page_base_lo + OFFSET`. A witness could therefore point a row at any address sharing the page's high limb and mint a second, forged history for it, breaking the one-entry-per-address property the offline memory-checking argument rests on. Reproduced end to end; see below. INIT must stay main-trace — it is the private input, and the verifier must not be able to recompute it. OFFSET has no such constraint: it is the dense `0..page_size-1` enumeration, byte-identical for every page regardless of program or input. Committing it alone binds exactly the column that must not be prover-chosen and publishes nothing. Approach: preprocess OFFSET only, rather than adding AIR constraints (`OFFSET[0] = 0` plus `OFFSET[i+1] = OFFSET[i] + 1`). The constraint route needs a real boundary constraint, and every VM table in this tree is built with `NullBoundaryConstraintBuilder` — there is no boundary machinery to follow, so that route means new infrastructure in the STARK layer. The preprocessed route instead reuses the mechanism that already runs on every proof for ELF-data and zero-init pages, and which `verifier.rs:1184-1213` already enforces. The bug was that private pages bypassed that check; the fix is to stop bypassing it for the one column that is public. It also costs no constraint degree and no constraint-evaluation time. Because OFFSET depends on neither program nor input, one commitment per blowup factor covers every private page, and the same value serves GLOBAL_MEMORY, whose OFFSET column is identical. Static constants follow the existing `static_zero_page_commitment` pattern (generated by `compute_static_commitments`, pinned by a drift test) with the same recompute fallback off the standard coset. Acceptance (full log in fix-acceptance.log): poc_control_honest_harness_verifies ... ok poc_negative_control_forged_run_without_repointed_row_fails ... ok poc_private_page_offset_forges_memory_contents ... FAILED panicked: SOUNDNESS HOLE NOT REPRODUCED: verifier rejected the forged proof The third failing is the point: that test asserts the forgery is ACCEPTED, and it passed on origin/main. The first passing is what shows the fix is not over-broad — honest proving still verifies. The PoC is converted into a regression test in the follow-up commit. * test(page): keep the OFFSET forgery as a regression test Inverts the PoC's central assertion now that the fix is in: the forged proof must be REJECTED. Renamed `poc_private_page_offset_forges_memory_contents` -> `forged_private_page_offset_is_rejected`, and rewrote the module doc, which still described the hole in the present tense. The two controls are unchanged and are what stop this becoming a test that passes for the wrong reason: `poc_control_honest_harness_verifies` fails if the fix breaks honest proving (a verifier that rejects everything would otherwise satisfy the assertion above), and `poc_negative_control_forged_run_without_repointed_row_fails` fails if the harness stops discriminating. Also drops two imports the fix made unused. * fix(verifier): validate and bound runtime_page_ranges before use `runtime_page_ranges` is a prover-chosen `VmProof` field with a free `u64` base and count, and `page_configs_from_elf_and_runtime` expanded it with a plain `for i in 0..count` push loop having validated nothing. The `expected_proof_count` cross-check that would reject a wrong page count runs *after* that loop, so it never got the chance: `RuntimePageRange { base: 0, count: u64::MAX }` made the verifier allocate `PageConfig`s until the process died — a verifier DoS on untrusted input. The function is now fallible and takes a `max_pages` cap enforced before and during expansion. The verifier passes `proofs.len()`: every page config needs its own sub-proof, so a layout wanting more pages than the proof carries can never verify. That makes the bound exact, needing no invented policy constant, and unable to reject anything an honest prover produces. Also validated up front, since all of it is attacker-controlled: - `count == 0`, which the honest run-length encoding never emits; - unaligned bases — which additionally keeps "same base" equivalent to "overlapping" for the duplicate check in the follow-up commit; - ranges running off the end of the address space, which the push loop would otherwise wrap in release. The overflow guard bounds the range's LAST BYTE, not its exclusive end. The stack's top page legitimately sits at the very top of the address space (`0xfffffffffffc0000`), where the exclusive end is exactly 2^64 and only the last byte is representable — bounding the end instead rejects every honest proof. A draft of this commit did exactly that; the PoC harness's honest control caught it, and `the_top_page_of_the_address_space_is_accepted` now pins it. New `Error::MalformedPageLayout`. Test call sites pass `usize::MAX` — they build layouts from honest data, not from a proof. * fix(verifier): reject two page tables covering the same address Second route to the violation the OFFSET binding closed, and this one needs no private input and no free column. `page_configs_from_elf_and_runtime` built a `Vec`, sorted it, and never deduped. So a prover declares `RuntimePageRange { base: <a real ELF .data page>, count: 1 }` and that address gets two PAGE tables: the ELF-data page with the real INIT, and a duplicate zero-init page. Both carry correct, verifier-recomputed preprocessed commitments — the duplicate matches the shipped `static_zero_page_commitment` exactly — so nothing is forged at the commitment layer, which is why pinning OFFSET does not touch it. Two genesis tokens then exist for every address in that page. The offline memory-checking argument needs the init set to hold exactly one entry per address; with two, the real page's row consumes the duplicate's token and the duplicate's row consumes the real one, and the bus balances while a value the program never wrote reaches a load. Every other row of the duplicate page self-cancels for free. `FINI`/`TIMESTAMP` are main-trace on every page, not just private ones, which is what lets the two rows swap which token each consumes. Reject rather than dedupe silently: a duplicate is never legitimate — the honest builder derives ELF pages from a `BTreeSet` and run-length-encodes the rest — so silent dedup would mask a prover bug instead of surfacing it. The check is a single adjacent-equality scan after the sort that already existed, which covers all three config sources at once (ELF, runtime, private) and so cannot be bypassed by adding a fourth. It relies on the alignment check from the previous commit to be a complete *overlap* check and not merely an equality one. Severity note: the OFFSET fix does limit this. The injected value is always `0`, since zero-init is the only page type a prover can conjure at an arbitrary base — so it forces a chosen address to read `0` at genesis instead of its real ELF byte. Still a forged execution (zeroing a length, a bound, a chain-id or a root byte suffices), but not an arbitrary byte at an arbitrary address. The framing: pinning `OFFSET` restores one row per address *within* a page; this restores one page per address. Both are needed. * test(page): end-to-end regression tests for both forgery routes Adopts the prosecutor's PoC harness (branch `poc/page-duplication`, 1bc1def6) wholesale rather than keeping my thinner copy, and inverts the assertions the way the OFFSET one was inverted. Their version is strictly better: it runs under PRODUCTION proof options (`GoldilocksCubicProofOptions::with_blowup(2)`, what public `verify` uses) instead of `default_test_options()`, and it carries two controls mine lacked. Eight tests, all passing, 24s: - `poc_control_honest_harness_verifies` — non-vacuity. The one that catches an over-broad fix; it already caught one (see the `runtime_page_ranges` commit). - `forged_private_page_offset_is_rejected` — route 1. Accepts refusal at either layer: `commit_main_trace` caches precomputed trees keyed by the expected root and skips the re-check on a hit, so a cold cache makes the prover refuse while a warm one leaves it to the verifier. Asserting one would be order-dependent. - `poc_negative_control_forged_run_without_repointed_row_fails` — the forged run without the compensating row must fail, so the harness discriminates. - `poc_negative_control_direct_init_tamper_on_preprocessed_page_fails` — rewrites INIT directly on the target's own ELF-data page. The bus balances perfectly, so the only possible rejector is that page's preprocessed commitment. It rejects: the mechanism works on ELF pages, and its absence on private ones was the whole of route 1. - `poc_real_ethrex_inputs_produce_private_input_pages` — reachability on the workload that matters. - `dup_structural_duplicate_page_coverage_is_rejected` — route 2's invariant in isolation: honest execution, every injected row self-cancelling, only the layout malformed. This is the one that flips pass→fail if the duplicate-base check is removed, and it cannot be satisfied by something incidental the way a forgery test might. - `dup_negative_control_without_compensating_row_fails` - `dup_duplicate_page_forgery_is_rejected` — route 2 end to end: ELF `.data` byte 0x11 read as 0x00, which was ACCEPTED against the unmodified ELF even after the OFFSET fix. A rejection now arrives in two shapes — `Ok(false)` from inside STARK verification, and `Err(MalformedPageLayout)` when the layout is refused before any proof is checked — so `verifier_accepts` collapses both and the tests do not have to care which fired. `craft_proof_with_duplicate_page` asserts the layout rebuild fails on duplicate coverage specifically, then still runs the full prove→verify path so the test stays end-to-end rather than degenerating into a unit test of the check. Also documents the test-only `minimal_bitwise` branch in `VmAirs::new`. That BITWISE AIR has no preprocessed commitment, so its lookup table would be prover-chosen — and since BITWISE backs `AreBytes`, an unpinned table would let a witness prove an arbitrary field element is a byte. It is safe only because all three production callers pass `false`; a fourth passing `true` would reintroduce the hole silently. The reconstruction-level tests in `page_layout_tests` stay: they cover shapes these do not (overflow, unaligned bases, count bounds, the top-of-address-space page). * test(page): tolerate prove-time refusal in the tamper regression tests CI failed on `poc_negative_control_direct_init_tamper_on_preprocessed_page_fails`: panicked at page_offset_forgery_poc.rs:455: this tamper leaves OFFSET alone, so the prover still builds it: PrecomputedCommitmentMismatch The `.expect` message was wrong on its own terms. The tamper does leave OFFSET alone, but it rewrites INIT on an ELF-data page — where the preprocessed columns are OFFSET *and* INIT (`NUM_PREPROCESSED_COLS = 2`). So it touches a preprocessed column after all, and `commit_main_trace` can reject it before a proof exists. Which layer fires is not deterministic. That function caches precomputed Merkle trees keyed by *the expected root* and skips the rebuild check on a hit (`crypto/stark/src/prover.rs:1161-1170`). A cold cache — a fresh CI runner — rebuilds from the tampered column and refuses; a warm cache — a local run that already proved something honest — substitutes the correct cached tree and lets the verifier do the rejecting. Local runs were warm, CI is cold. Both outcomes are rejections, so the test now accepts either via a shared `proof_or_prover_refusal`, which still requires an `Err` to be specifically `PrecomputedCommitmentMismatch` rather than any proving error. The test's meaning is unchanged: it pins that the preprocessed commitment rejects a direct INIT rewrite, which is what shows route 1 was that mechanism's *absence* on private pages rather than a flaw in it. `forged_private_page_offset_is_rejected` now shares the same helper instead of its own inline match. Swept the rest of the file for the same assumption. The rule, now documented on `Tamper`: a tamper touching a PREPROCESSED column may be refused at prove time and must go through the helper; one touching only main-trace columns cannot be and may keep `.expect(..)`. By that rule the three remaining `.expect`s are sound, and each now says why rather than asserting it: - the honest control — no tamper at all; - the uncompensated forged run — the forged execution moves FINI/TIMESTAMP (main trace) while OFFSET/INIT still come from the honest ELF; - duplicate-page injection — writes FINI only. Verified both orderings: 8/8 serial (warm cache, verifier path exercised), and each rejection test passing alone in a fresh process (cold cache, the CI path). * Fix/page offset review followups (#910) * drop the accidentally committed fix-acceptance.log' * docs(page): fix a doc comment on the wrong fn --------- Co-authored-by: jotabulacios <jbulacios@fi.uba.ar>
* Add on-demand hint ecall (host-computed) * Add HINT prover table for the hint ecall * Add hint ecall guest tests and test programs * Route ecsm inverses and sqrt through hint ecall * Make the hint ecall ABI big-endian * Validate the Hint ecall operand addresses * Verify hints by difference instead of byte compare * Bind HINT writes to x12 and range-check bytes * Fix hint doc placement and guest cargo config * Verify hints with a mandatory software fallback * Constrain the HINT multiplicity column as boolean * Drop BENCH-ONLY labels from the hint ecall * Test that IS_BIT rejects a non-boolean HINT mu * Run ethrex-crypto host tests in CI * Add software fallback and test seam to field_inv * GPU parity-check the HINT table * Move HINT syscall off the FEXT_FMA numberD * Bind and range-check the HINT ecall operands * lint * Fix stale hint-ecall comments (#899) - executor/Cargo.toml: drop the BENCH ONLY label on the k256 dep. 515a921 removed those labels everywhere else; compute_hint is production executor code reached by real ecrecover proofs. - hint_min: the ethrex call site is aligned, not unaligned — get_hint in crypto/ethrex-crypto wraps its output in an align(8) buffer. * Correct the hint_min alignment comment The guest doc claimed the ethrex call site is unaligned, but ethrex-crypto's get_hint wraps its output in an align(8) newtype precisely to keep the four HINT writes on the MEMW_A path — a bare [u8; 32] on the stack is only 1-aligned. Someone trusting the comment and dropping the wrapper would add four wide MEMW rows per hint call, on every ecrecover. * Drop the BENCH ONLY label from the k256 dependency k256 is on the prove path, not only in benchmarks: the trace builder's collect_hint_ops recomputes every hint's output with compute_hint because the value is not carried in the CPU log. A maintainer trusting the label and feature-gating the dependency away would break proving. * Range-check the HINT output address low limb, like the input one The HINT table range-checked in_addr's low limb on the ALU bus but left out_addr to the memory bus, reasoning that an output address straddling the 2^32 limb boundary cannot balance. The bus does bound it, but only to 2^32 - 25: the write bases are out_addr_lo + 8i, so the largest one stops being a canonical limb at 2^32 - 24, while MEMW's carry columns resolve the bytes past it correctly. The executor rejects anything above 2^32 - 32 with HintAddressOverflow, which left the seven-value window 2^32-31 ..= 2^32-25 that the AIR accepted and the executor halts on — a prover could prove a hint call the VM rejects. Send the same LT range-check for out_addr's low limb. The existing in_addr bound is reused unchanged, since 2^32 - 31 is exactly addr_limb_ok(addr, 31) for either operand, and is renamed HINT_ADDR_LIMB_BOUND now that it covers both. The trace builder emits the matching LT op, and the sizing pass counts three LT rows per hint call instead of two — LT is an upper-bound table there, so the count only has to stay >= the built trace, which is why the count_table_lengths drift test does not catch an undercount on its own. Tests assert that both address columns carry an ALU LT sender against that bound, and that the bound accepts exactly the limbs addr_limb_ok accepts, with the seven-value window as an explicit regression. * Derive the HINT selector bound from the executor's accepted set HINT_SELECTOR_BOUND was a literal 3 in the prover, while the executor decided validity with matches!(hint_id, HINT_FIELD_INV | HINT_SCALAR_INV | HINT_FIELD_SQRT). Nothing linked the two, so appending a fourth selector would make the HINT table assert LT(selector, 3) = 1 against an LT row the builder emits as 0 — an unbalanced ALU bus with no algebraic pointer to the cause. Move the bound next to the selectors it bounds, express the ecall's rejection as is_valid_hint_selector, and const-assert that every selector below the bound is valid and that the bound itself is not. The prover re-exports the bound instead of restating it, so a selector added without moving the bound fails to compile rather than surfacing as a bus imbalance at proving time. * ci(executor): run the executor lib unit tests The unit tests under `executor/src/tests/` live in the lib target (`#[cfg(test)] pub mod tests;` in lib.rs), so none of the `--test <name>` steps select them, and the `test_ckzg` step filters by name and runs only ignored tests. They therefore never ran in CI — including the hint ecall's `HintUnknownSelector` / `HintAddressOverflow` / per-selector coverage, which has no other home. The new step shares the lib test binary with the `test_ckzg` step, so it costs a test run rather than an extra compile. * test(ethrex-crypto): cover the negated-sqrt and canonical-but-wrong hints The existing lying-hint tests all feed `[0; 32]` / `[0xFF; 32]`, which die in `Scalar::from_repr` / `FieldElement::from_bytes` and never reach the verify predicate. So the checks the fast paths' soundness actually rests on — `(x * inv) == 1` and `x·inv - 1 == 0` — had no test that exercised their rejecting branch. - `field_inv` / `scalar_inv`: hints that parse cleanly and simply are not the inverse (`inv + 1`, `-inv`), which must be rejected and recomputed. - `decompress_r`: an oracle returning the *other* root. That is not a lie — `-y` is as valid a root of x³+7 as `y` — so the verify accepts it and the fallback never runs, leaving the parity-selection branch solely responsible for the sign. With the honest oracle that branch fires only for the `k` whose root happens to have the wrong parity; forcing the negation exercises it for every `k`. Also drops a dangling "property C1" reference from the module doc and states the property directly. * test(hint): exercise all three selectors in the hint_multi guest The guest called `HINT_FIELD_INV` three times, so the AIR's `selector < 3` range-check was only ever exercised at 0 — an accepted-value bound that no end-to-end test pushed against. One call per selector (`HINT_FIELD_INV`, `HINT_SCALAR_INV`, `HINT_FIELD_SQRT`) covers the whole accepted range; `sqrt`'s input is 4, a quadratic residue mod p, so the hint is a real root rather than the zeros `compute_hint` returns on a numeric failure. `test_prove_hint_multi_rust_guest`'s expected value follows, now computed through `compute_hint` per selector instead of assuming three field inverses. * test(hint): pin the guest's selector constants against the executor's `is_valid_hint_selector` and its const-assert tie the AIR's range-check to the executor's accepted set, so the prover and executor can no longer disagree. The *guest* is a third declaration and is still unbound: `lambda-vm-syscalls` re-declares the same three selectors as `usize`, in a crate the workspace excludes, linked to the executor's `u64` copies by nothing but a comment. A divergence there is silent. The ecall would either trap on an unknown selector, or — worse, for a value that stays in range — return the wrong function's answer, which the guest's verify-then-fallback swallows as "the host lied" and quietly recomputes in software. Nothing fails; the guest just runs ~2000x slower for the right result. `lambda-vm-syscalls` is added as a dev-dependency for it. Unlike `crypto/crypto`'s and `ethrex-crypto`'s copies it is not target-gated, so it does build on the host — safe because that crate's guest-only items (the `#[global_allocator]` and the `_start`/`main` entrypoint) are already `cfg(target_arch = "riscv64")`, and `executor::tests` is itself `#[cfg(test)]`, so the non-test lib build never links it. * docs(hint): correct three comments the operand work left stale Follow-on to "Range-check the HINT output address low limb" and "Derive the HINT selector bound", which added interactions and constants but left these behind. - `hint.rs`: the `HintConstraints` doc still said the LogUp argument "already fixes `mu`'s value via the timestamp-unique `Ecall` tuple", framing `IS_BIT` as belt-and-braces. That contradicts the module doc directly above it: the `Ecall` tuple carries a per-instruction timestamp, a free column, so LogUp pins only the *sum* of `mu` over rows sharing a tuple — which a witness can satisfy by spreading `mu` with integer weights summing to 1. `IS_BIT` is load-bearing, and the doc now says so and points at that argument. Its bus list was also stale (one register read, no LT senders); it is three and three. - `prover/src/test_utils.rs`: same stale bus surface on `create_hint_air`. - `crypto/ethrex-crypto/src/lib.rs`: the comment justifying `negate(y2)` over `negate(rhs)` claimed negating `rhs` "would silently compute the wrong value in release". That is not what happens. k256's `negate(magnitude)` computes `2*(magnitude+1)*P_limb - self` under a `debug_assert!(self.magnitude <= magnitude)`; for a magnitude-2 operand the result stays non-negative, so the value is correct and it is the debug assert that fires. The reason to prefer `negate(y2)` is real, but it is a build-configuration hazard, not a wrong answer — worth stating accurately in a comment that exists to explain a non-obvious choice. * ci(ethrex-crypto): run the hint tests in release too, not only debug k256 0.13.4 swaps its FieldElement implementation on `debug_assertions` (arithmetic/field.rs): debug selects the magnitude-tracking `field_impl` wrapper, release selects the raw `FieldElement5x52`. The guest ELF is built with `cargo build --release`, so every hint-verification test was exercising an implementation the guest never compiles -- and `test-ethrex-crypto` was the only test step in pr_main.yaml without `--release`. The two builds are not interchangeable for these tests. `ConstantTimeEq` differs between them: the debug wrapper compares the magnitude and normalized tags alongside the limbs, the release type compares limbs only. A magnitude-contract violation would panic loudly in the tested build and compute a silently wrong value in the shipped one. Keep both: release is what ships, and debug's magnitude asserts turn a contract violation into a panic rather than a wrong answer. --------- Co-authored-by: MauroFab <maurotoscano2@gmail.com> Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com>
Kimi Code ReviewAutomated review by Kimi (Moonshot AI) |
Codex Code ReviewNo issues found in the specified PR diff. The spec structure validator also passes. |
…clines, close an R2 corruption race (#914) * fix(gpu): recover device-only tables by downloading the resident LDEs on an R2 miss The device-only gate is a static predicate over a dynamic dispatch: it cannot mirror every reason the device R2 path might decline (parts count, kernel eligibility, transient errors, shapes a new workload brings), and each miss was a hard abort that deadlocked the epoch pipeline — DECODE on the synthetic workload, then a second table on the real-block bench. Instead of excluding tables one by one, treat the resident handles as the source of truth: on a miss, download the main/aux LDEs back to host, clear the device-only flag, and continue on the host path. Slower for that table, never wrong; the abort remains only when the handles themselves cannot serve the data. gpu_device_only_downgrades() counts recoveries so a persistently-missing condition still gets mirrored into the gate. * fix(gpu): drain-and-retry, then host downgrade, for resident-aux LDE declines A transient CUDA OOM on the resident aux LDE was a hard prove failure: the resident build leaves no host aux trace to fall back to. A device drain releases the concurrent VRAM peaks, so one retry usually keeps the table fully resident; if it still declines, download the resident aux trace (and the main LDE when the table is device-only) and continue host-backed. The drain before dropping the resident buffer also keeps kernels enqueued by the failed attempt from reading pool memory reused by a concurrent table. * fix(gpu): serialize the device R2 window to close a transient H corruption race Concurrent device R2 windows under VRAM pressure can transiently produce a fully wrong H for one or two tables while every input stays correct (rerunning the same chain on the same resident inputs matches the host), yielding a proof that fails the composition check. Serializing only the constraint-eval + decompose window across tables eliminates it; commits and host arms stay parallel, and the windows overlap rarely enough that the lock is near-free. LAMBDA_VM_GPU_SERIALIZE_R2=0 lifts the lock to bisect further or once the underlying race is found. * fix(gpu): device-only requires the d=2 composition path The device R2 path only exists for the d=2 quotient split; a table with any other composition bound (DECODE proves with num_parts == 1) would skip it entirely and hard-abort on its device-only trace. * fix(gpu): keep already-present host buffers in the downgrade recovery A mixed state (one commit fell back to CPU while the other stayed device-only) left the recovery refusing to proceed: it treated a missing device handle as fatal even when that side already had a valid host copy. Only the missing side is downloaded now, and the R3 host-arm guards check the buffer they are about to read instead of the table-wide flag. * chore(gpu): drop an orphaned diagnostic helper download_ext3_columns came along in a cherry-pick but its only consumer (the cross-check post-mortem) ships separately; dead code under the cuda feature. * fix(gpu): run the host decompose of a downloaded H outside the R2 lock The fallback arm (download H, host iFFT + LDEs) executed under the serialization lock, so under VRAM pressure — exactly when that arm runs — it serialized every other table's device window behind pure CPU work. The lock now covers only the device eval + decompose + the H download; the host decompose and every host arm run outside it. The lock is also acquired only for d=2 tables (the others never enter the device path). * chore(gpu): review follow-ups on the downgrade recovery set_host_data had been inserted between set_num_rows' doc and its signature, stealing its doc comment and un-gating it from the cuda feature; the serialization lock now recovers from poisoning instead of cascading PoisonErrors over the original panic; the downgrade counter joins reset_all_gpu_call_counters and the device-only residency test asserts it stays at zero on the happy path; stale comments about the aux gate mirroring the main gate rewritten with the actual contract. * fix(gpu): harden the downgrade downloads, parallelize the recovery transposes (#921) * fix(gpu): harden the downgrade download recovery Three fixes on the device-only downgrade path, all in the graceful degradation function whose whole point is to avoid a hard abort. - The aux branch of `materialize_lde_trace_host` sliced the downloaded slabs without checking their length, so a short download would panic inside the recovery instead of degrading. Both sibling download paths already validate (`download_main_lde_row_major` checks `col_major.len() != m * lde`, `materialize_aux_trace_host` checks `raw.len() != rows * cols * 3`); this adds the matching check. - Restore the `len/capacity % 3` guard the other two ext3 `from_raw_parts` sites carry, spelled `is_multiple_of` because clippy's `manual_is_multiple_of` rejects the older form here. - The failure error claimed "host aux trace is empty" on a path where that is false: when the aux download succeeded and the follow-up main-LDE download failed, the host aux trace had just been populated. Track which recovery step failed and name it. Control flow unchanged. * perf(gpu): parallelize the downgrade recovery transposes Both conversions in the recovery path were single-threaded nested loops over the full LDE: the col-major -> row-major main transpose in `download_main_lde_row_major`, and the de-interleaved-slabs -> row-major interleaved aux conversion in `materialize_lde_trace_host`. For MEMW at LDE 2^20 those are a 411 MB and a 327 MB buffer respectively, walked with a strided access on one core. Both now follow the existing idiom in `trace.rs` ("Parallel col-major -> row-major transpose"): parallelize over OUTPUT row chunks with `par_chunks_exact_mut`, so every element is still written exactly once and no unsafe is involved. The index math is unchanged -- chunk `r` of width `m` is `row_major[r * m + c]`, and chunk `r` of width `m * 3` sub-chunked by 3 is `interleaved[(r * m + c) * 3 + k]` -- because the layout was verified against the kernels. Gated on the `parallel` feature with the sequential loop kept for builds without it, and skipped when `m == 0` since `chunks_exact_mut(0)` panics. These loops run on a scheduler driver thread holding no locks, so rayon is safe here, unlike the pinned-staging unpack in math-cuda. * docs(gpu): align device-only and downgrade docs with the recovery semantics (#920) This branch turned two of the device-only hard-aborts into downloads that recover and continue host-backed, but the surrounding docs still describe the old contract: "every host read hard-aborts", "the prove aborts loudly", "a mis-gate panics one of the guards". Rewrite those to say what the code now does — R2 and the R1 resident-aux commit recover and bump GPU_DEVICE_ONLY_DOWNGRADES, R3/R4 still abort, and the R3 guards check the individual buffer so mixed states are legal. Also correct the R2 lock comment (it serializes submission, not execution, for device-only tables), note that the numeric gate is not the complete predicate on its own, broaden the downgrade counter's doc to cover resident-aux declines on tables that were never device-only, and drop the false "only" from materialize_lde_trace_host's failure list. Comments, doc comments, two assertion message strings and one doc-comment run command (--test-threads=1, matching the Makefile target). No behavior changes. * chore(gpu): read the R2 serialize env var directly The OnceLock cache bought nothing — the var is consulted a handful of times per prove and does not change over the binary's lifetime. * fix(gpu): cap the GPU test targets, count the aux retries, split the downgrade counter (#924) * ci(gpu): cap each GPU prover test target with a wall clock A panic on a device-only cliff assert can leave the prover hung instead of aborting: the panicking thread unwinds while its siblings stay parked in CUDA driver waits, so the process never exits. Observed repeatedly on rented 5090s under VRAM pressure. The merge-queue GPU job runs the Makefile targets through scripts/gpu_test.sh with no per-target limit, so one hang holds the box until the workflow timeout kills the whole job with no indication of which group stalled. Wrap the four cuda targets that run the prover in `timeout -k 30 2700`. 45 minutes is well above their normal runtime and well below the job timeout, and timeout's 124 exit fails the target, so gpu_test.sh names the stalled group and the merge is blocked. test-math-cuda is left alone: kernel parity never enters the prover. * feat(gpu): count the resident-aux drain-and-retry The R1 resident-aux path retries the device LDE after a full device drain when the first attempt declines, and that retry usually succeeds — which is the problem: a successful retry left no trace anywhere except an eprintln, so how often the device actually declines under VRAM pressure was unmeasurable in production, where nobody is reading stderr. GPU_RESIDENT_AUX_RETRIES makes the decline rate observable and separates it from its consequence: retries with no downgrades means the drain absorbed the pressure, while the two rising together means the drain is no longer enough. * fix(gpu): split the downgrade counter by site GPU_DEVICE_ONLY_DOWNGRADES counted two unrelated events: the R2 device-only downgrade in materialize_lde_trace_host, which is always a device-only gate miss, and the R1 resident-aux downgrade in materialize_aux_trace_host, which is entered whenever aux_resident() is set and so fires on tables the gate never marked device-only. A GPU run made that concrete: a preprocessed BITWISE table took the R1 downgrade despite never being device-only, and the combined counter reported it as a gate miss with nothing to distinguish it from one. Keep GPU_DEVICE_ONLY_DOWNGRADES on the R2 site alone and add GPU_RESIDENT_AUX_DOWNGRADES for the R1 site, so a nonzero value names its own fix: mirror the missing condition into the gate for the former, relieve VRAM pressure for the latter. The integration test now asserts both are zero with per-site messages, and the gate docs say which counter each round bumps. --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
* Add opt-in dlmalloc guest allocator * Use dlmalloc as the default guest allocator * Fix and test the dlmalloc bump provider * Regenerate guest program lockfiles * Default the guest allocator to bump * Drop the TLSF guest allocator * Regenerate the ethrex-tests lockfile * Regenerate the guest lockfiles left stale by the ChaCha20 removal * docs * Correct the guest allocator's documented claims * fix(syscalls): review follow-ups for the bump allocator default Mechanical follow-ups on the allocator swap. No behaviour change on any path that runs today; the one code change closes a failure mode that is currently prevented by a linker flag rather than by anything in this file. benchmark-pr.yml missed syscalls. The push-to-main paths filter listed prover, crypto, executor, bin/cli, tooling/ethrex-fixtures and the Makefile, but not syscalls -- so a change landing only in syscalls, which is exactly what this branch is, would not refresh main's benchmark baseline. syscalls is linked into the guest ELF, so an allocator swap moves cycles on every workload; main's baseline would have stayed stale until some prover file happened to change, and until then the comparison guard would have suppressed the table. pr_main.yaml:99 already hashes 'syscalls/**' into the guest-ELF cache key, so the two workflows disagreed about what rebuilds the guest. Two lockfiles still carried embedded-alloc. crypto/ethrex-crypto and tooling/ethrex-block-converter are detached workspaces with their own Cargo.locks, which is why the sweep missed them: both still listed embedded-alloc under lambda-vm-syscalls after syscalls/Cargo.toml stopped declaring it. Regenerated via cargo metadata in each workspace. The only removals are embedded-alloc's own transitive tree (const-default, linked_list_allocator, rlsf, and in ethrex-crypto also rustversion, svgbobdoc, base64 0.13, syn 1.0.109, unicode-width); no other package's version moved. The 10 added lines are all ` "syn",` losing its version-disambiguation suffix now that only one syn remains. bench_vs/sp1/fibonacci/Cargo.lock also names embedded-alloc, but that is sp1-zkvm 6.0.1's own dependency and is left alone. imp::init is now idempotent in both arms. Both arms stored HEAP_POS unconditionally, so a second call rewound the cursor back over live allocations. With alloc_zeroed's memset removed -- sound only because bump never re-serves a region -- the next alloc_zeroed would then return dirty bytes, and the guest would compute on garbage while the prover produced a perfectly valid proof of that wrong execution. No crash and no diagnostic, so it is worth a guard rather than a comment. HEAP_END serves as the initialized flag (init_allocator always passes a nonzero MAX_MEMORY_SIZE), a debug_assert makes a double call loud in debug builds, and the host tests gain a #[cfg(test)] reset() since they deliberately re-point the global cursor at their own heap. Worth stating why this could not happen already, because the reason is not the call sites: all six guests that call init_allocator() explicitly also override the ELF entry with `-C link-arg=-e -C link-arg=main` in their .cargo/config.toml, so _start -- the only other caller -- never runs for them, and guests entering through _start never call it explicitly. The safety rested on an entry-point flag; a guest that dropped `-e main` while keeping its explicit call would have rewound. Three comment corrections and one warning. - The dlmalloc dep comment called it the allocator to pick "for continuations". Wrong criterion: continuations are a prover-side split of a single guest execution and change nothing about what the guest allocates. The criterion is a guest whose cumulative allocation has no per-execution bound, which is how src/allocator.rs already frames it. - allocates_zeros()'s comment described an "mmapped marker" that dlmalloc may set. There is no marker bit: Chunk::mmapped(p) is `(*p).head & INUSE == 0`, the absence of both in-use bits (dlmalloc 0.2.14 src/dlmalloc.rs:1805). The old comment's "the Rust port has no mmap path, so nothing is ever mmapped" is also not quite true -- init_top (dlmalloc.rs:789) writes a segment-end sentinel with head = top_foot_size() = 80 on 64-bit, and 80 & INUSE == 0, so that sentinel is mmapped()-true (harmless: never returned to a caller). Replaced with the durable argument: every path that returns a pointer to a caller goes through set_inuse / set_inuse_and_pinuse / set_size_and_pinuse_of_inuse_chunk, all of which set CINUSE, and calloc_must_clear is only ever evaluated on a user pointer, so no user chunk is ever mmapped. Consequence the old comment omitted: calloc_must_clear is therefore always true, calloc always memsets, and allocates_zeros() == true is inert -- not a performance win, kept only for correctness-by-construction should upstream grow an mmap path. - The comment on the bump arm's checked_add claimed the overflow is unconstructible from the Layout invariant alone. It is not: Layout gives size <= isize::MAX - (align - 1), which with aligned <= pos + align - 1 bounds aligned + size <= pos + isize::MAX, and that is < 2^64 only if pos < 2^63. The missing half is that alloc stores new_pos only when new_pos <= HEAP_END, so pos <= HEAP_END = 0xC000_0000. The checked_add stays -- it keeps the argument local to alloc instead of resting on both halves. - New note on the DLMALLOC static: an initialized Dlmalloc is address-sensitive and must never be moved. smallbin_at returns a pointer into self.smallbins and init_bins writes self-pointers into that array, so relocating it after first use (into a Box, a OnceCell, or a local) silently corrupts the bins. Safe as a static; the note is for whoever refactors it. Verified: syscalls tests pass on both arms -- 9 passed on the default bump arm (5 allocator + 4 keccak) and 12 on --features dlmalloc-alloc (8 allocator + 4 keccak). cargo fmt --check and cargo clippy --all-targets clean on both arms (the two surviving warnings are pre-existing manual_is_multiple_of in src/keccak.rs:104-105). benchmark-pr.yml parses and its paths list resolves to the seven expected entries. * Grow the top bump block in place on realloc * Trigger the hyperfine bench on syscalls changes * Test the allocator init guard in both profiles * Replace the bump ceiling claim with measurements * fix doc * fix a comment * Drop the TLSF reference from the allocator proof test's doc This PR removes the TLSF heap, so the test no longer exercises TLSF init. It proves the same program against whichever allocator is built in, so name the step rather than the implementation. Comment-only. --------- Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com> Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com> Co-authored-by: MauroFab <maurotoscano2@gmail.com> Co-authored-by: Nicole <nicole.graus@lambdaclass.com>
* perf(prover): default the cuda table scheduler to K = num_airs
`table_parallelism()`'s cuda arm scaled K by `available_parallelism()`
(`cores * 2 / 3`). Measured over 881 runs on two RTX 5090 boxes, that is the
wrong shape. All eight core-count curves fit `T(K) = S + max(Tmax, W/K)` within
run-to-run noise, and the work K divides — W ≈ 5.3-8.0 s — is invariant to host
core count over an 8x range, to CPU model, and to rayon pool width: cutting
RAYON_NUM_THREADS 32 -> 4 leaves W alone and merely doubles S, with the best K
still num_airs at every pool width. `available_parallelism()` sizes precisely
that rayon pool, so it is the wrong quantity to scale K by. K is not a thread
count; each table's work runs on the one global pool.
Worst case against the best measured K, over four core counts on both boxes:
cores/3 +30.2 %
cores*2/3 +13.0 % (what this replaces)
constant 12 +7.0 %
num_airs +1.6 % (both non-zero cells inside noise, p = 0.88 / 0.80)
`cores*2/3` fails where it was predicted to: low core counts, K=2 at 4 cores
(+13.0 %) and K=5 at 8 cores (+8.1 %).
Taking the ceiling rather than solving for an optimum is right in both regimes
of the fit: if W/num_airs > Tmax more K strictly helps, and if W/num_airs < Tmax
the extra drivers are floor-limited and cost nothing — the one staging slab is
held 56 % of wall at K=31 and wall time still improves. The old doc comment's
mechanism ("in-flight tables mostly sit in GPU waits") is not what happens —
mean GPU utilisation never exceeded ~38 % at any K — so it is rewritten rather
than re-tuned. What is meant to bound concurrency is memory admission rather
than a count: that is what VramGate is for, and it never binds at the default
budget.
`table_parallelism` now takes `num_airs` and clamps to it, replacing the
`.min(num_airs)` the call site applied. `auto_storage::decide` keeps a bounded
figure through the new `storage_estimate_parallelism()`: `peak_bytes` sums the
transient bytes of the top-k tables, so an unbounded k there sums every table —
measured +27 % at 128 PAGE tables, +44 % at 512 — and would spill proofs to disk
that fit in RAM. Its value is unchanged, so no storage decision moves.
The CPU arm keeps `cores / 3`. The sweep ran only on cuda builds, where the
parallelized work is device-bound; on a CPU-only build every table is pure host
work and none of this evidence transfers.
* docs(stark): compress the table_parallelism doc comment
The sweep record moves out of the tree to a gist linked from PR #911, and
the full defense of the K = num_airs choice (curve fit, rayon-width legs,
per-cell p-values) lives there and in the PR body. The code site keeps the
conclusion, the mechanism in one sentence, the headline numbers, and the
pointer.
* fix(stark): satisfy unnecessary_lazy_evaluations on the cuda clippy pass
Under the cuda feature the unwrap_or_else closure in table_parallelism
collapses to a plain num_airs, tripping the lint on the Makefile's cuda
clippy pass. Move the cfg split outside the closure: the cuda arm uses
unwrap_or, the CPU arm keeps its lazy host_cores() call.
---------
Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com>
…R4 DEEP, comp-tree, R3 barycentric) (#935) * fix(gpu): recover device-only declines at the remaining cliff sites (R2 commit, R3 OOD, R4 DEEP) Under VRAM pressure a device dispatch can decline after the device-only gate already skipped the host drain, and the host fallbacks at the R2 comp-poly commit, the R3 parts/trace OOD and the R4 DEEP loop hard-abort on the empty host buffers. Download the resident data instead: the trace LDEs via materialize_lde_trace_host, the H part evaluations via a new download off the resident R2 parts handle. The asserts remain only for handles that cannot serve the data. The R4 DEEP host loop reads both the trace and the part evals, so it recovers both sides. Also adds sticky fault-injection hooks (test-faults) to the cuda barycentric, DEEP and comp-tree entries: the drain-and-retry absorbs one-shot faults, so the cliff paths need a fault that keeps firing. * test(gpu): exercise the cliff-site recoveries end to end Three prove+verify runs under sticky faults (comp-tree, barycentric, DEEP), each requiring the device-only path to fire on the warm-up and the recovery counters to move. * fix(gpu): address cliff-recovery review — race-free sticky hook, parallel parts download Review follow-ups on the device-only cliff recovery: - check_sticky: collapse the load-then-decrement into one fetch_update that saturates at 0, so concurrent per-table dispatches can't underflow the counter — which would break both the sticky guarantee and the `== 0` fired check. - cuda_fallback_tests: disarm the sticky faults with a Drop guard, so a panic in prove or a failing assert can't leave one armed and cascade into the next test in the single-threaded binary. - download_composition_parts_host: de-interleave under rayon and reinterpret the u64 buffer in place, matching materialize_lde_trace_host instead of copying again through u64_to_ext3_vec — this path fires often under VRAM pressure. - Docs: the device-only downgrade counter now also covers transient device declines, not only gate misses; note the new &mut contract on get_trace_evaluations_from_lde. * style(gpu): rustfmt check_sticky and correct its doc cargo fmt collapses the aligned match-arm comments (the CI lint failure); also drop a stale doc sentence describing an earlier post-load variant that the fetch_update version does not use. * test(gpu): assert the parts-download counter is zero on the happy path (#938) The device-only cliff recoveries replace hard aborts with a silent download-and-continue, so the counters are now the only thing that surfaces a gate/dispatch lockstep break. GPU_DEVICE_ONLY_DOWNGRADES (trace side) already has its == 0 guard here; its parts-side counterpart did not, and its only readers were the > 0 assertions in cuda_fallback_tests, which run with a fault deliberately armed. Without this, a decline in the R2 comp-poly tree build on a device-only table recovers, verifies and passes green, while every such table pays a full parts D2H plus a CPU commit_bit_reversed and loses the resident composition tree. The R4 DEEP site is already covered transitively (it needs the trace to be device-only too, which moves the trace counter), so this closes the R2 commit and R3 parts-OOD sites. Zero is the right expectation: materialize_composition_parts_host early-returns without bumping when the part evals are already populated, so the counter only moves for a device-only table that had to pull its parts back. The message names both causes rather than blaming the gate, matching the counter's own doc, which now allows a transient VRAM decline as well as a gate miss. --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
* ci(bench-gpu): stop building on half-provisioned or bad-RAM boxes The GPU ABBA bench kept failing on rented Vast boxes in ways that looked like code bugs but were the harness building before the box was ready: - The provisioning-complete check fell back to "these few artifacts exist" and started the build while onstart was still populating the sysroot, so the C compiler read a half-written header (truncated bits/timex.h -> "unterminated #ifndef"). Require the "=== done ===" marker only; drop the premature fallback. - Add a toolchain sanity gate (trivial gcc + rustc compile) after provisioning: a bad-RAM host that SIGSEGVs the compiler on the first heavy crate (jemalloc, serde_derive) now fails fast here with a clear message instead of mid-build with an internal-compiler-error backtrace. - Cap the dual build at CARGO_BUILD_JOBS=8 so the initial ramp (LLVM codegen units + jemalloc's nested make -j) can't transiently exceed the box's RAM and trigger OOM-induced compiler crashes. - Filter offers by reliability>=0.95 to skip chronically-flaky hosts before renting (fails safe: over-strict just yields no offers). A full box-reroll (rent another host on a build/prove failure) is the next step but needs a live run to validate against paid infra, so it is left out of this change. * fix(bench-gpu): make the toolchain gate able to fail, and say why (#940) * fix(bench-gpu): make the toolchain gate able to fail, and say why Follow-ups from review of the provisioning hardening. - The sanity gate could not fail on a compiler failure. Under `set -e` a non-final operand of an `&&` list is exempt from errexit, and the list's non-zero status does not re-trigger it, so a dead cc/rustc was swallowed and the remote exit status was that of the trailing `rm -rf`. The gate returned 0 and printed "toolchain sane" on a host whose compiler had just crashed. Measured, before -> after: cc SIGSEGV 0 -> 139, cc missing 0 -> 127, cc error 0 -> 1, rustc SIGSEGV 0 -> 139, rustc missing 0 -> 127, healthy 0 -> 0. Every command is now a bare statement; a trap keeps the tmpdir cleanup on both paths. - Distinguish ssh's own exit 255 from a verdict on the toolchain, so a network blip no longer reports the host's compilers as broken. - Run the probe from the repo so rustup resolves the pinned toolchain in rust-toolchain.toml rather than whatever default the image carries. - A failure in this step posted "Run failed" above an EMPTY code block: the PR-comment step tails $RUNNER_TEMP/abba_out.txt, and only the bench step ever wrote it. Record the reason and the compiler output there. - Reword the gate's error. It establishes "cc or rustc could not compile and run a trivial program"; bad RAM is named as one possible cause rather than asserted as the diagnosis. Comments, each previously at odds with the code or with each other: - the gate blamed bad RAM while the CARGO_BUILD_JOBS comment blamed memory pressure for the same symptom. The latter now describes OOM as it actually presents (SIGKILL, or an allocation failure) and names jemalloc-sys's CARGO_MAKEFLAGS forwarding, which is what makes the cap bind its nested make. - drop the unmeasured "~10 min dual build", and annotate the 3 min 56 s ETA reference as a pre-cap measurement that CARGO_BUILD_JOBS=8 will raise. - the no-offer error and the env header now list reliability, gpu_frac and cuda_max_good, which they had drifted from. - state the gate's scope: it does not exercise /opt/lambda-vm-sysroot, and a 1 s compile surfaces marginal RAM only sometimes. * fix(bench-gpu): tell the operator to wait before re-rolling the box Both host-fault messages said "Re-run /bench-gpu to reroll the box", but offer selection is deterministic — `sort_by(.dph_total) | reverse | .[0]` with no machine_id exclusion — so an immediate re-run can re-pick the same machine once it relists and fail identically. Say to wait a few minutes instead, and say why, so the advice matches what the picker actually does. The ssh-255 message is left as an immediate retry: a transport failure is not a verdict on the host, so there is nothing to roll off. Still not an automated reroll (the sibling gpu-tests.yml carries a TRIED machine_id list for that); this only stops the message promising something the selection logic does not do. --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
Collaborator
|
Quick review:
|
* perf(gpu): grind the proof-of-work nonce on the GPU Grinding (generate_nonce) runs a ~2^grinding_factor parallel Keccak search per table per epoch and is the prover's dominant CPU cost — 64.7% of on-CPU time in a 100tx flamegraph, on the 16 cores while the GPU sits ~66% idle. Add a keccak nonce-search kernel (each thread strides a nonce block, atomicMin keeps the smallest valid nonce), a math-cuda wrapper that searches in expanding blocks from 0, and a stark dispatch that computes the inner hash on the host, validates the device result unconditionally, and falls back to the CPU search on any device miss or invalid nonce. Result-valid: the verifier only checks is_valid_nonce, so any valid nonce works. A device launch is skipped below a minimum grinding factor (tiny factors are faster on the CPU), and LAMBDA_VM_NO_GPU_GRIND forces the CPU path. GPU_GRIND_CALLS counts the dispatches so a silent fallback is caught by the integration test. 100tx e20 (ABBA, same binary): 18.89s -> 13.10s = -30.6%. * fix(gpu): review follow-ups on the GPU grinding PR (#945) Route the GPU dispatch and its tests through one inner-hash-to-lanes conversion. The tests built their own copy, so the line the prover actually runs was executed by nothing: swapping it to from_be_bytes would have kept every test green while is_valid_nonce rejected every device nonce at runtime and the search sat on the CPU fallback forever. stark::grinding:: inner_hash_lanes is now the single entry point, which also lets get_inner_hash go back to private. Report that fallback on stderr instead of log::warn. The CLI initialises env_logger with no default filter, so a warn-level line never prints unless RUST_LOG is set — and it is the only signal that the kernel has started returning garbage. The other device-decline paths already use eprintln with a [gpu] prefix. Wrap test-math-cuda in GPU_TEST_TIMEOUT. It was the only one of the five GPU targets without it, and it is Group 1 of gpu_test.sh, so a hang there costs Groups 2-5 as well and a job timeout yields `cancelled`, which skips the run-summary step and leaves no readable output. Document LAMBDA_VM_NO_GPU_GRIND in the profiling README's knob list. Drop the "Parity" framing from the test module: there is nothing to be at parity with, since any valid nonce is acceptable and the CPU's find_any does not agree with itself between runs. What is pinned is validity, plus the search completeness that minimality stands in for — noted as a probe rather than a contract, so a future kernel that deliberately returns any valid nonce relaxes the assertion instead of being treated as broken. Same for the doc on generate_nonce_maybe_gpu, which claimed "smallest" for both arms. --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
…-size tables (#888) * add profiling * fix(profiling): field fixes from the first sessions on the 5090 box - run_profile.sh: nsys export needs --force-overwrite (nsys stats already materializes the sqlite); tolerate runs that produce no timeline JSON - flamegraphs.sh: fixed off-CPU capture window sized from the on-CPU run (SIGINT through sudo is unreliable and produced 0-byte captures); find offcputime-bpfcc in /usr/sbin (Debian) - bench_mode.sh: set the CPU governor via sysfs when cpupower is absent - setup_machine.sh: Debian-aware perf install (linux-perf); extract libnvToolsExt from the cuda-nvtx-12-8 deb into ~/nvtx (CUDA >= 12.9 removed NVTX v2 from the toolkit) with LAMBDA_VM_NVTX_LIB override - docs: benchmark/profiling examples use ethrex 5tx/10tx fixtures only (team convention: never fibonacci); plan status updated * docs(profiling): complete the toolkit README as a reference Adds the pieces needed to use the tooling without reading the scripts: column-by-column semantics for phase_table.md and phase_busy.md (including NVML gpu% vs nsys busy% and launch-site attribution), a reference table of every script with its flags, the environment variables the tooling understands plus the pre-existing prover knobs for A/B experiments, and a troubleshooting section (missing NVTX ranges, silent CPU fallback, empty off-CPU captures, jitter, concurrent-thread span nesting). * perf(gpu): async pinned D2H + pre-created event pool + precomputed-tree cache The optimization half of the original campaign commit, without its profiling layer (this branch keeps gpu-profiling-tooling's toolkit as the only instrumentation): - async_dtoh_via/PendingD2H: big D2H copies go through per-worker pinned slabs via raw cuMemcpyDtoHAsync + a reusable completion event, instead of cudarc's memcpy_dtoh whose pageable path blocks the calling thread for all prior stream work (host DtoH blocking 12.4s -> 6.7s on the original ethrex A/B). - GpuLdeBase/GpuLdeExt3 carry a 'ready' event; consumers wait device-side (cuStreamWaitEvent) instead of producers host-synchronizing. - Events are pre-created at backend init plus a reusable pool: a mid-prove cuEventCreate convoys the driver lock (~30ms/call measured under load). - Precomputed-column Merkle trees are cached process-wide keyed by their commitment root, so preprocessed tables (DECODE/BITWISE/range) stop rebuilding identical trees on every prove; only the multiplicity columns are recommitted. * perf(prover): pipeline + concurrent epoch proving in continuations Producer thread executes and builds epoch i+1's traces while epoch i proves; K epoch provers (LAMBDA_VM_EPOCH_CONCURRENCY, default 3) consume prepared epochs concurrently — epoch proofs are mutually independent (label-domain-separated transcripts), results re-ordered by index so proof bytes match the sequential schedule. The DECODE commitment is computed once per continuation prove instead of per epoch. Same as the original campaign commit minus its epoch-timeline instrumentation (this branch keeps the profiling toolkit's spans as the only instrumentation; they are re-homed onto this pipelined flow at the end of the series). * perf(gpu): dim-split constraint interpreter with liveness-reused value slots The constraint interp/composition kernels evaluated every IR node as ext3 and kept one global-memory scratch slot per node, so scratch size and traffic scaled with program length (KECCAK_RND/ECSM/ECDAS at full thread count needed 26-39 GB, failing the alloc and silently falling back to CPU via result.ok()). Lowering (constraint_ir/device.rs) now assigns dim-split slots: - Base-dim nodes compute in the base field (1 mul vs 9 for ext3) and live in u64 slots (8B vs 24B); mixed base*ext ops use mul_base / componentwise shortcuts that are bit-identical to the full ext op on the embedded operand (SUB components keep the literal sub(0, x) form, which is NOT bitwise neg on non-canonical limbs). - Slots are liveness-reused (linear scan, freed at last use, roots pinned), so per-thread scratch is the max-live-set, not the node count: 8-35x smaller across the 26 tables (CPU 14.4KB -> 1.3KB, ECDAS 596KB -> 17KB per thread). Scratch allocs drop the memset. - Row-invariant leaves (constants, RAP challenges, alpha powers, table offset) are propagated into operand encodings (kind<<29|payload) and never touch scratch; they only materialize when a root needs them. The CPU walker eval_device_program mirrors the new walk and stays the pre-GPU parity oracle; the 26-table differential vs the production folder and the on-GPU parity tests (synthetic + all real programs) pass bit-for-bit. ir_stats_dump (ignored) prints per-table node/slot stats to size scratch when tuning. Measured on RTX 5090 (nsys, ethrex): constraint_composition_kernel 814ms -> 267ms (-67%) over the same 29 launches; ethrex 10tx continuations ABBA 15.16s -> 14.77s. * perf(gpu): commit preprocessed tables through the fused GPU pipeline Preprocessed tables (DECODE/BITWISE: precomputed + multiplicity column split) skipped the fused GPU commit entirely — commit_main_trace only tried the GPU when precomputed.is_none() — so they paid the CPU row-major LDE plus two CPU subset Merkle trees (~2.2s thread-time of R1 'Main commit Merkle CPU' on ethrex). - keccak256_leaves_base_row_major_row_pair_range: column-range variant of the row-pair leaf kernel, byte-identical to the CPU commit_rows_bit_reversed_subset layout. - coset_lde_row_major_split_trees: one row-major GPU LDE of all columns plus the two subset trees built on device; both node buffers download to host and rebuild full host trees via from_precomputed_nodes, so the preprocessed opening path, the process-wide precomputed-tree cache and disk-spill work unchanged. The shared expansion stage is factored into expand_row_major_on_stream (same code path as the existing fused commit). - The table now gets a GpuLdeBase handle (column-major LDE + trace snapshot, no device tree), so its rounds 2-4 (composition, DEEP, barycentric) run on GPU too. Preprocessed openings short-circuit to the host trees via is_preprocessed, as before. - REGISTER stays on CPU (LDE below the dispatch threshold). Parity: split_tree_tests pins roots and opening paths against the CPU subset commits on device; cross-binary verification of full ethrex bundles passes both ways. Measured on RTX 5090: ethrex 10tx continuations interleaved 3-way 14.77s -> 14.23s (cumulative -6.1% vs the pre-kernel baseline). * perf(prover): overlap the global prove with the epoch proves' tail prove_global consumes only execution artifacts — the per-epoch cell boundaries built by the producer, the ELF and the genesis pages — never an epoch proof, yet it ran serially after every epoch prove finished (~0.9s of pure tail on ethrex 10tx). The producer now publishes each epoch's boundary (an Arc share of the one already flowing to the epoch provers — no data copy) on a dedicated channel, in epoch order. A scoped thread drains that channel until the producer hangs up (last epoch prepared) and proves the global memory argument while the tail epochs are still proving. On an epoch failure first_err still wins and the global result is discarded; proof bytes and bundle content are unchanged — only the schedule moves. The epoch timeline confirms the tail is gone: the global prove runs fully inside the window of the last three in-flight epoch proves. Measured on RTX 5090: ethrex 10tx continuations ABBA 14.16s -> 13.66s (-3.5%); cross-binary verification passes both ways. Day cumulative across the three optimizations: -9.4% (15.16s -> 13.66s). * perf(prover): share per-ELF DECODE artifacts across continuation epochs Every epoch's trace build re-parsed the ELF and regenerated the pristine DECODE trace (~1M rows) inside the serial producer chain, plus moved a ~900K-entry pc->row map by value per epoch. DecodeArtifacts (instruction map + pristine DECODE trace + pc->row index) is a pure function of the ELF: prove_continuation builds it once and every epoch's build clones the pristine trace (a memcpy) and fills its own multiplicities; build_traces now borrows the pc->row map. The monolithic entry point delegates and is unchanged. Net work removal with identical trace bytes (cross-binary verification passes). Wall-neutral within noise on a 32-core box; groundwork for pipelining the epoch trace build out of the producer chain, where parallel builders would otherwise each redo the ELF parse. * perf(prover): pipeline epoch trace builds onto a builder pool The continuation producer built every epoch's full trace tables inline, so the serial chain feeding the provers was execute + collect + BUILD per epoch (~95% of it table generation) — 7.2s of a ~18s wall on a 32-core box, with the last epochs' proves gated on it. The epoch trace build is now split at its real sequential boundary: - Traces::collect_epoch (Phases 1-2): op collection over the advancing memory image — stays on the producer, in epoch order. - Traces::build_from_collected (Phases 3-5): table generation — pure epoch-local work, runs on a small builder pool (LAMBDA_VM_TRACE_BUILDERS, default 2) between the producer and the epoch provers, bounded channels capping peak memory. The cross-epoch chain no longer touches traces: the boundary derives from CollectedEpoch::touched_memory_cells (same function, same immutable memory_state as the build) and the next epoch's register init from register::fini_from_final_state — a trace-free mirror of the REGISTER FINI column, pinned by fini_from_final_state_matches_trace. PAGE tables are the build's only image consumers and continuation mode skips them, so builders need no image snapshot. Measured on a 32-core RTX 5090 box (ethrex 10tx continuations): the producer chain drops 7.2s -> 2.9s and the first three proves start ~1s earlier, but the wall ties (~18s) — the box is bound by total CPU work, which this change conserves (proves and the global dilate to absorb the freed schedule). A K/builders sweep confirms K=3/B=2 stays optimal. Expected to pay on wider boxes where idle cores can absorb the parallelism; groundwork for cutting per-epoch CPU work (AIR/capture caching), which is the binding constraint on narrow boxes. * perf(prover): cache pre-captured AIR prototypes per table type Constructing an AirWithBuses runs every constraint body through a MetaBuilder, and the first constraint_program() runs them again for the IR capture — for ECDAS/ECSM/KECCAK_RND (16-25K IR nodes) that dominates AIR construction (0.78s per VmAirs::new on ethrex). Continuation epochs rebuild the full AIR set per epoch and shard tables build one instance per shard, so the same walks re-ran dozens of times per prove. build_air now keeps a process-wide prototype cache keyed by (table name, proof options): the prototype is built and pre-captured once, and every later request clones it — Clone on AirWithBuses copies the derived meta, LogUp layout and the captured IR inside the OnceLock, never re-running the bodies. PAGE stays correct because its page base is part of its name. with_name/with_preprocessed apply to the caller's clone; the cached prototype stays pristine. Wall-neutral within noise on the 32-core box (the removed work is a few core-seconds against a ~580 core-second prove); cross-binary verification passes both ways. Also cuts AIR construction out of the monolithic path and the test suites. * profiling: re-home the toolkit spans onto the pipelined continuation flow The toolkit's continuation instrumentation assumed the sequential epoch loop. With the producer/builder/prover pipeline the stages run on different threads, so the spans move to where the work actually happens: - prove_continuation_total root span + timeline reset at entry, drained at the end exactly like the monolithic path (stdout tree + LAMBDA_VM_TIMELINE_JSON for phase_table.py). - epoch_execute / epoch_collect on the producer, epoch_trace_build on the builder pool, epoch_prove on the prove workers — each prove/build/ collect also opens an NVTX range with per-epoch identity (epoch_*[i=N]) for Nsight timelines. - Spans close BEFORE blocking channel sends, so backpressure waits are never booked as work. - prove_global span on the overlapped global-prove thread. * perf(prover): cache constraint-program lowering and share captured IR across clones * perf(prover): cache domain-derived values process-wide Domain and LdeTwiddles are now shared across epochs and concurrent epoch provers via a process-wide cache keyed by (field, trace_length, blowup, coset_offset). The OOD barycentric constants, FRI inverse twiddles, and the d=2 decomposition inverses hang off them as lazy per-domain values instead of being rebuilt (each an LDE-size-order batch inversion or clone) per table per epoch. * perf(prover): dedup boundary-zerofier inverses per (domain, step) Each boundary constraint paid its own LDE-size batch inversion even when sharing the step with its neighbours, and the vectors are identical for every table and epoch on the same domain. The inverted vector now lives in the shared domain, keyed by step, and constraints hold an Arc to it. * perf(gpu): keep boundary-zerofier columns resident on device Upload each distinct column once (GpuBaseVec, cached keyed by its host Arc — storing the Arc pins the allocation so the key can never alias) and D2D-copy into each dispatch's flat buffer, instead of re-uploading tens of MB per table per epoch over PCIe. * perf(gpu): keep the d=2 composition pipeline on device The composition evaluations stay resident after the fused kernel; a pointwise kernel decomposes them into the H0/H1 slabs, the batched slab LDE extends both halves with no H2D, and the parts handle feeds R4 DEEP. One drain of the final evaluations (still read by the commit tree and the query openings) replaces four codeword-sized PCIe trips per table per epoch. Falls back to downloading H and running the host decompose on any device failure. * perf(gpu): fold FRI directly from the device-resident DEEP codeword The fully-resident DEEP arm keeps its output on device, bit-reverses it into FRI order with a permutation kernel, and hands the buffer to the FRI fold state as its working codeword — removing the download / CPU-bit-reverse / re-upload round trip. The commit loop is shared between the host and device entries and restores the transcript on any mid-loop failure so the CPU path reruns cleanly. * fix(prover): keep lazy domain-cache initialization off the rayon pool The shared domain caches ran the parallel batch inversion inside their OnceLock initializers. A rayon worker that starts such an initialization farms chunks to the pool while sibling workers block on the same cell; with every worker parked the chunks never run and the prove deadlocks (observed as a full-process futex stall). Initializers now use the sequential inversion, and domain construction pre-fills every lazy cell from the setup thread so pool workers never run — or wait on — an initializer mid-prove. * chore(gpu): drop the unused DEEP download bridge and silence clippy * fix(prover): drain the epoch pipeline on error instead of stranding its senders The prove/build channel receivers live in the outer scope, so a worker that returned on error left the bounded senders parked in send() with no consumer — any mid-run proving error hung prove_continuation forever instead of surfacing. Workers now drain-and-discard until the channels disconnect, the producer stops executing epochs once an error is recorded, and the global-prove thread skips its (whole-prove-sized) run when the bundle can no longer be assembled. * fix(gpu): harden device-path edge cases from review - PendingD2H now synchronizes on drop: an error between enqueue and wait no longer releases the pinned slab to reuse/free while the DMA is in flight. - domain_and_twiddles re-checks the cache under the insert lock so a build race can't pin a duplicate instance's columns in the pointer-keyed device caches. - Hard-assert b_z_inv column length at the D2D copy (a short column left uninitialized VRAM in the kernel's window), mirror the batched-LDE input asserts in the split-trees entry, gate mismatched FRI twiddles to the CPU path, and pin the ext3 tower in the shared FRI drive. - Refresh the event-tracking safety note to the wait_ready_on contract. * test(prover): cover the epoch pipeline's mid-run error path A builder-injected fault (keyed by a magic private input, so it is stateless and inert for every real caller and for concurrent tests) fails epoch 3 of a ~9-epoch prove — enough pending work past the bounded channels' slack that a shutdown regression wedges instead of returning. The test runs the prove under a timeout so that regression fails CI rather than hanging it. * chore: fix profiling doc drift, untrack pycache, drop inert braces - The per-entry-point NVTX shape ranges were dropped when the math-cuda pipelines were rewritten; four doc sites still promised them and the nsys report mislabeled its innermost-range table. Align them with what the nvtx feature actually emits (mirrored instruments spans). - Untrack scripts/profiling/__pycache__ and ignore Python bytecode. - Remove ~86 brace wrappers in math-cuda left inert by the async-DMA refactor (kept the ones that scope real borrows) and reword three comments that referenced a deleted sync label. * style: cargo fmt * chore: keep working notes out of the tree * refactor(prover): prove continuation epochs on a single worker * chore: sync recursion bench lockfile with ecsm's num-integer dep * new opt * fix(gpu): harden round-2 residency paths after review Grid-stride the fused row-major NTT past gridDim.y (lde >= 2^24 silently fell back to CPU), assert the device-only contract in the R2 composition commit and preprocessed opening fallbacks, validate htod_via bounds, retain FRI device evals only under device-only, and move the inverse fault-injection hook so every batch-inverse entry is covered. * perf(prover): replace table chunks with a VRAM-admitted per-table scheduler Fiat-Shamir only requires the main roots absorbed in index order before the shared challenges; past that fork every table's chain is independent. Phase A now runs all main commits under a byte-budget admission gate (no chunk barriers), and aux build, aux commit and rounds 2-4 run fused as one task per table, heaviest first — while a big table works through a host-bound stretch, the other tables' GPU stages fill the device. GPU builds default TABLE_PARALLELISM to 2/3 of the cores (swept flat at 10 on a 16-core RTX 5090). ethrex 10tx continuations on RTX 5090: 10.64s -> 8.54s (-19.7%, 8 ABBA pairs). * perf(gpu): device-only preprocessed tables, lower LDE threshold, PCIe hygiene - Extend the device-only gate to preprocessed tables (BITWISE/DECODE): the split-trees path takes retain_host_lde, R4 openings serve both subsets from the device row gather (multiplicity range + precomputed range), and the is_preprocessed exclusion is gone. - Default GPU LDE threshold 2^19 -> 2^14: CPU-committed mid tables had no device handle, so every R2-R4 dispatch re-uploaded their LDE per round. - Multi-eval-point chunked barycentric kernels for R3 OOD (one pass over the LDE for all eval points, cols x chunks grid) with per-point fallback. - Device cache for domain coset points keyed by (len, p0, p1). - Pre-upload big main traces from the epoch builder thread; the R1 commit D2D-copies instead of paying the H2D in its chain. BITWISE is excluded (prove_epoch edits its multiplicities post-build) and update_multiplicities drops any stale pre-upload defensively. - scripts/profiling/h2d_histo.py: memcpy attribution histogram by NVTX phase and transfer size from an nsys sqlite export. * chore(gpu): clippy manual_range_contains on the bary multi asserts * chore(gpu): allow too_many_arguments on the split-trees wrapper * fix(gpu): keep the aux D2H when the GPU main commit fell back A static device-only gate on the aux commit could mark the trace device-only with no main GPU handle to serve it, turning a recoverable CPU fallback of the main commit into a hard abort downstream. * build(gpu): single-source the barycentric eval-point cap BARY_MAX_K (kernel accumulator array) and BARY_MAX_EVAL_POINTS (dispatch assert) were defined independently; build.rs now defines both from one constant, so they cannot drift into kernel stack corruption. * fix(gpu): verify the coset-cache invariant, cap trace pre-upload by VRAM budget The device coset cache keys on (len, p0, p1), which only determines the contents for a geometric sequence — verify it at sampled indices on insert. The builder's pre-uploaded traces ride ahead of the admission gate, so cap them to a slice of the device budget instead of competing with the prove peak on small cards. * fix(gpu): decouple the device-only envelope from the GPU commit threshold Lowering the commit threshold to 2^14 silently widened device-only to every mid-size table. The gate cannot mirror kernel-side dispatch eligibility, so a single R2 decline on one of those tables hard-aborts the prove (seen at 100tx once main's keccak rework landed) and deadlocks the epoch pipeline. GPU commits and resident handles keep paying from 2^14; dropping the host copy stays at the proven 2^19 envelope (LAMBDA_VM_GPU_DEVICE_ONLY_THRESHOLD overrides). * fix(gpu): device-only requires the d=2 composition path; name the table in the R2 abort The device-resident R2 path only exists for the d=2 quotient decomposition. DECODE proves with a single part, so admitting it to device-only skipped the whole device path and hard-aborted into the empty host trace, deadlocking the epoch pipeline at 100tx. Mirror the parts count in the gate, and include the table identity in the abort message — finding this one took a live-process backtrace because the message did not say which table died. * fix(gpu): default the trace pre-upload off Wall-neutral on the 5090 (the scheduler already hides the H2D) and its riding-ahead buffers sit outside the VRAM admission gate: at epoch 2^22 the real-block prove peaks at ~23 GiB and the extra 4 GiB pushed it into CUDA_ERROR_OUT_OF_MEMORY. Opt-in via LAMBDA_VM_TRACE_PREUPLOAD_MB. * fix(gpu): recover device-only tables by downloading the resident LDEs on an R2 miss The device-only gate is a static predicate over a dynamic dispatch: it cannot mirror every reason the device R2 path might decline (parts count, kernel eligibility, transient errors, shapes a new workload brings), and each miss was a hard abort that deadlocked the epoch pipeline — DECODE on the synthetic workload, then a second table on the real-block bench. Instead of excluding tables one by one, treat the resident handles as the source of truth: on a miss, download the main/aux LDEs back to host, clear the device-only flag, and continue on the host path. Slower for that table, never wrong; the abort remains only when the handles themselves cannot serve the data. gpu_device_only_downgrades() counts recoveries so a persistently-missing condition still gets mirrored into the gate. * feat(gpu): in-process cross-check diagnostics for device-side corruption LAMBDA_VM_GPU_XCHECK runs the verifier's composition consistency check inside the prover after round 3, per table at negligible cost; on a failure a post-mortem recomputes each device stage on host, reports the corruption shape, reruns the device chain to tell a transient race from a corrupted resident input, and aborts. LAMBDA_VM_GPU_FORCE_DOWNGRADE exercises the device-only R2 recovery end to end. The R2 downgrade path now names the table it recovered. A proof_diff ignored test structurally diffs two continuation bundles. * fix(gpu): drain-and-retry, then host downgrade, for resident-aux LDE declines A transient CUDA OOM on the resident aux LDE was a hard prove failure: the resident build leaves no host aux trace to fall back to. A device drain releases the concurrent VRAM peaks, so one retry usually keeps the table fully resident; if it still declines, download the resident aux trace (and the main LDE when the table is device-only) and continue host-backed. The drain before dropping the resident buffer also keeps kernels enqueued by the failed attempt from reading pool memory reused by a concurrent table. * fix(gpu): serialize the device R2 window to close a transient H corruption race Concurrent device R2 windows under VRAM pressure can transiently produce a fully wrong H for one or two tables while every input stays correct (rerunning the same chain on the same resident inputs matches the host), yielding a proof that fails the composition check. Serializing only the constraint-eval + decompose window across tables eliminates it; commits and host arms stay parallel, and the windows overlap rarely enough that the lock is near-free. LAMBDA_VM_GPU_SERIALIZE_R2=0 lifts the lock to bisect further or once the underlying race is found. * fix(gpu): keep already-present host buffers in the downgrade recovery A mixed state (one commit fell back to CPU while the other stayed device-only) left the recovery refusing to proceed: it treated a missing device handle as fatal even when that side already had a valid host copy. Only the missing side is downloaded now, and the R3 host-arm guards check the buffer they are about to read instead of the table-wide flag. * test(gpu): exercise the forced-downgrade recovery end to end LAMBDA_VM_GPU_FORCE_DOWNGRADE declines every device R2 path so each device-only table goes through materialize_lde_trace_host and finishes on the host evaluator; the test proves a small ethrex fixture with a lowered device-only threshold, asserts the downgrade counter moved and that the proof verifies. Wired into the test-cuda-fallback group. * fix(gpu): run the host decompose of a downloaded H outside the R2 lock The fallback arm (download H, host iFFT + LDEs) executed under the serialization lock, so under VRAM pressure — exactly when that arm runs — it serialized every other table's device window behind pure CPU work. The lock now covers only the device eval + decompose + the H download; the host decompose and every host arm run outside it. The lock is also acquired only for d=2 tables (the others never enter the device path). * chore(gpu): review follow-ups on the downgrade recovery set_host_data had been inserted between set_num_rows' doc and its signature, stealing its doc comment and un-gating it from the cuda feature; the serialization lock now recovers from poisoning instead of cascading PoisonErrors over the original panic; the downgrade counter joins reset_all_gpu_call_counters and the device-only residency test asserts it stays at zero on the happy path; stale comments about the aux gate mirroring the main gate rewritten with the actual contract. * chore(gpu): read the R2 serialize env var directly The OnceLock cache bought nothing — the var is consulted a handful of times per prove and does not change over the binary's lifetime. * style(gpu): drop needless refs in the coset-geometric assert * fix(gpu): review follow-ups on the round-4 residency PR (#937) Wrap the new gpu_force_downgrade target in GPU_TEST_TIMEOUT. That variable exists because a device-only cliff panic leaves the prover hung rather than aborting, holding the rented merge-queue box until the workflow timeout, and this target is the one that deliberately drives every device-only table through the decline path. Correct three comments that overstate or misdescribe what the code does: - DEFAULT_DEVICE_ONLY_MIN_LDE promises mid tables "degrade to CPU instead of aborting". That holds for the sites that read the LDE, which all gate on host_trace_empty(), but not for the R4 Merkle-proof gather: the host tree is root-only for every GPU-committed table whatever retain_host_lde says, so a declined gather has nothing to fall back to. Lowering the commit threshold widens that one abort site even though the device-only envelope is unmoved. - The new is_root_only assert claims the host walk would emit an empty path for position 0. get_proof_by_pos refuses root-only trees, so it panics instead — the assert's value is naming the cause, not preventing a bad proof. - gather_proofs_dev says callers fall back to the host tree on None. All three call sites .expect() and abort. Note that DEFAULT_GPU_LDE_THRESHOLD gates the whole dispatch layer, not just the commit, so moving it moves R2/R3/R4/FRI together. Document the four new env vars and h2d_histo.py in the profiling README, which is the toolkit's reference. Pin bary_num_chunks' three branches with unit tests, and cover the 64-chunk cap in the kernel parity tests — every existing case is rows-bound at 1-2 chunks, including the one annotated as exercising the occupancy branch. * fix flaky test --------- Co-authored-by: Diego K <43053772+diegokingston@users.noreply.github.com> Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
* perf(gpu): run DECODE (num_parts=1) DEEP/FRI on device Extend the device-resident composition-parts path to num_parts==1. For d=1, H is already the single part on the LDE coset, so deinterleave it into a 1-part GpuLdeExt3 (comp_h_to_slabs_ext3 kernel, no NTT) instead of running DEEP/FRI on host; the commit, R3 OOD, R4 DEEP, FRI and openings already read the part count from the handle. Proof-identical; host fallback preserved. * test(gpu): cover the num_parts==1 (DECODE) device path; assert d=1 invariants - add prover/tests/cuda_d1_path.rs + `make test-cuda-d1` (gpu_test.sh Group 3): lowers LAMBDA_VM_GPU_LDE_THRESHOLD so DECODE engages the d=1 device DEEP/FRI path end to end, asserting gpu_comp_h_slabs_calls > 0 and the proof verifies. Its own binary because gpu_lde_threshold() caches the env (OnceLock). - decompose_comp_h_dev: debug_assert want_host and H rows == LDE domain size on the d=1 branch (the d=2 arm gets an incidental check via weights.len()). - try_deinterleave_comp_h_dev: document that the always-drained host part feeds the release query-0 canary — the only e2e check on the d=1 layout. * test(gpu): make test-cuda-d1 actually exercise DECODE (#947) `test-cuda-d1` set LAMBDA_VM_GPU_LDE_THRESHOLD=64 on the premise that 64 is "the exact LDE size of fib_iterative_1M's DECODE ROM". It is 32, and the reasoning behind the number was wrong too: DECODE's rows come from the ELF's executable words, not from cycles. fib_iterative_1M is 13 executable words (one 52-byte executable PT_LOAD; the variants differ only in the `li a0, <count>` immediate, so fib_iterative_16M is 13 too). 13 + 1 CPU-padding entry = 14 -> next_power_of_two() = 16 rows -> blowup 2 -> DECODE LDE 32. At threshold 64 that is below the gate, so DECODE failed the R1 split-tree commit, had no gpu_main() handle, and evaluate_dev declined - DECODE never reached the d=1 path at all. The counter could therefore only be fed by KECCAK_RC, the only other num_parts==1 table (a d=1 table is one with a single bus interaction), whose fixed NUM_ROWS=32 gives LDE 64 and passes `64 < 64` by one unit. So the target, the test name, the module docs and the assert message all named the one d=1 table guaranteed not to be exercised. No threshold fixes this with a fib fixture: DECODE (32) sits below KECCAK_RC (64), so <=32 engages both and 33..=64 engages only KECCAK_RC. Switch to all_instructions_64 - 66 executable words -> 128 rows -> DECODE LDE 256 - at threshold 128, where DECODE engages with 2x margin and KECCAK_RC declines, so a nonzero gpu_comp_h_slabs_calls() uniquely attributes to DECODE. 128 is also higher than the previous 64, so strictly fewer tables land on the GPU-committed path: it narrows rather than widens the R4 gather_proofs_dev abort site that gpu_lde.rs warns about for lowered thresholds. Tighten the test's own guard while here. `thr > 0 && thr < 1<<14` passed vacuously for any wrong value - including the 64 that caused this - so pin the window to (KECCAK_RC_LDE, DECODE_LDE] against named constants instead. * docs(gpu): correct the d=1 composition-parts comments and stale group counts (#948) * docs(gpu): update the group counts the new test group invalidated Adding cuda_d1_path as Group 3 of gpu_test.sh renumbered the groups after it, but five references still describe the old five-group layout: - scripts/gpu_test.sh: "the prover suite (Groups 4 & 5) proves asm AND rust guests" is now Groups 5 & 6 - and it is the only thing explaining why the script builds rust guests up front, so a Group 5 failure sends the reader to test-cuda-fallback, which needs no rust guests. - Makefile: a hang in Group 1 now costs Groups 2-6, not 2-5. - gpu-tests.yml: the group enumeration and "5 test groups" both predate the new group; that comment is the merge-gate contract for anyone who does not open the shell script. - cuda_path_integration.rs: the R2 composition-LDE comment enumerates two num_parts arms; there are now three, and the new one increments neither counter in the assertion below it (the assertion is still correct - no d=1 table here crosses the default threshold - so this is comment-only). Also move the coverage note off the end of check_composition. It described suite-wide coverage from inside a helper shared by two tests, and its claim that the end-to-end d=1 counterpart "is not asserted ... exercised by real-program proves (ethrex) and the GPU bench instead" was invalidated by this branch's own second commit, which adds prover/tests/cuda_d1_path.rs. Restate it accurately on the decode-shaped test it actually describes. * docs(gpu): correct the d=1 composition-parts comments; share the admission gate Four claims in the new d=1 prose do not match the code. 1. "all of which already read the part count from `handle.m`", and the same in decompose_comp_h_dev's doc. Only the R2 commit and the R4 openings read handle.m. R3's z^P exponent and R4 DEEP's gamma count read lde_composition_poly_evaluations.len() - the host part Vec's length - and DEEP merely validates the handle against it, declining on a mismatch. FRI never receives the handle at all. Benign today, because the d=1 arm always drains one host part, but the sentence is the stated reason for not touching R3/R4 and it credits the handle with the host Vec's authority. Replace it with the invariant that actually has to hold - handle.m == lde_composition_poly_evaluations.len(), which materialize_composition_parts_host also requires - and note the same on the d=2 arm's doc. 2. "it is the only end-to-end check that the device m=1 gather / DEEP / FRI layout is correct". The canary compares a device composition-row gather against the host part evals; DEEP and FRI consume separate downstream buffers and are not covered by it. cuda_d1_path.rs already describes the same canary correctly, as guarding "the composition-row gather". Narrow the claim to the in-prove gather check and point at proof verification for DEEP/FRI. 3. "zeroing a preprocessed table's host trace fails its commitment check". Preprocessed tables do go device-only: commit_main_trace takes device_only, the caller applies no preprocessed exclusion, and the preprocessed branch passes !device_only as want_host precisely to support it. The precomputed-root check runs against the device-built tree, so the host drain cannot reach it, and host_trace_empty is not set until Round1 construction - after every R1 commit. Nothing is zeroed either; the Vec is left empty. Restore the accurate reason (any other part count has no device R2 path and needs the host evaluator) and give d=1's real one: it always drains its single part to feed the canary, so it gains nothing from dropping the host trace. 4. "the degree gate below" in decompose_comp_h_dev. There is no gate below it in that function; the gate is device_only_for, far above. Also drop the duplicated half of device_only_for's rationale, which restated the d=2 sentence eight lines later and was where claim 3 lived, and correct the "nothing to unwind" note on the d=1 download ordering: both values drop by RAII in either order, so the ordering is about keeping the blocking D2H off the tail of the de-interleave launch, not about unwinding. Two small cleanups while in here: - Hoist the admission gate the d=1 and d=2 producers had duplicated verbatim (two TypeId guards plus the threshold/power-of-two test) into dev_comp_parts_gate, so a future condition - a VRAM check, a tower widening - cannot land on only one arm and silently diverge them. - Rename try_deinterleave_comp_h_dev to try_comp_h_to_slabs_dev, matching the kernel (comp_h_to_slabs_ext3), the math-cuda entry point (comp_h_to_slabs) and the counter (GPU_COMP_H_SLABS_CALLS); it was the one link in that chain that a grep from either end would miss. Drop the single-use `decomposed` temporary at the call site, which read as a borrow workaround where none is needed. --------- Co-authored-by: Mauro Toscano <12560266+MauroToscano@users.noreply.github.com>
|
Benchmark Results for modified programs 🚀
|
Collaborator
Author
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Specs the affine ECSM variant added in #879.
An
is_affineselector lets one chip serve both ecalls: it picks the ECALL number the chip answers to, and gates theyGread (addr_xG + 32, atts) and theyRwrite (addr_xR + 32, atts + 3). Everything else is shared. Two checks come with it — theyGread pins the input point's parity, which becomes observable onceyRis published, andyR < pforces the output canonical.ECSM goes 37 → 42 variables, 708 → 757 columns.
Numbering change: the ecall number is now
-11 - 2·id - is_affine, so secp256r1 moves from -12 to -13. #879 ships affine secp256k1 at -12, and secp256r1 has no implementation.Not spec'd: #879's address-limb LT bounds. Those close a gap in
ecsm.rs, which builds dword bases in the low limb only; the spec derives every address with a full 64-bitADD.