A Code Review Caught a Real API Footgun. Here's the Fix.

A function took a program and a trace, and quietly ignored the program. Nothing in this repo ever misused it — but nothing in the type system stopped a future caller from doing so, either.

2026-08-31 · crates/zkvm-stark/src/lib.rs

An external review of this repo flagged a specific function signature as a footgun:

pub fn public_inputs_for(program: &Program, trace: &ExecutionTrace) -> PublicInputs {
    let _ = program; // kept for API symmetry with call sites
    PublicInputs {
        initial: trace.rows.first().map(|r| r.acc).unwrap_or(BaseElement::ZERO),
        result: trace.result,
        program: trace.rows.iter().map(ProgramRow::from).collect(),
    }
}

The claim: this takes a program argument and does nothing with it — every field of the returned PublicInputs comes from trace alone. A caller could pass a program and an unrelated trace, and the function would happily build public inputs that don't actually correspond to that program.

Verifying it before touching anything

Before changing a soundness-adjacent function on the strength of a pasted claim, we checked it directly: read the source, confirmed the let _ = program; line, then searched every call site in the workspace. All four looked the same:

let padded = program.padded();
let trace = execute(&padded);
let pub_inputs = public_inputs_for(&padded, &trace);

In every real call site, trace is derived by executing the exact same program passed alongside it, immediately beforehand. So the claim's second half checked out too: this was never a live bug in this codebase — every caller happened to maintain the invariant the function itself didn't enforce.

The fix

Rather than adding a comment warning callers not to misuse it, the signature changed to make the misuse impossible to express:

pub fn public_inputs_for_program(program: &Program) -> PublicInputs {
    let trace = zkvm_isa::execute(program);
    PublicInputs {
        initial: trace.rows.first().map(|r| r.acc).unwrap_or(BaseElement::ZERO),
        result: trace.result,
        program: trace.rows.iter().map(ProgramRow::from).collect(),
    }
}

No trace parameter means no mismatched-trace argument is possible. All four call sites simplified too — each one dropped its own execute() call and trace variable, since the new function does that internally.

Verifying the fix, not just the diff

A signature change to a function on the verification path needs more than "it compiles." Before considering this done:

• Full workspace test suite (33 tests at the time) — passed under --release
• A real zkvm prove → zkvm verify round trip against an actual example program, not just unit tests — passed
• cargo clippy --release --workspace --all-targets -- -D warnings — clean
• Pushed and watched through the real GitHub Actions CI run rather than trusting the local pass — every example program still proved and verified against a live server

The pattern worth taking from this: a footgun that's never been fired isn't nothing — it's a bug waiting for the next caller who doesn't already know the unwritten rule. Fixing the type instead of the documentation is usually the smaller, more permanent change.