Skip to content

fix(zkboost): fill mock_proving_time defaults from slot duration#1401

Open
barnabasbusa wants to merge 1 commit into
mainfrom
bbusa/zkboost-mock-proving-time-defaults
Open

fix(zkboost): fill mock_proving_time defaults from slot duration#1401
barnabasbusa wants to merge 1 commit into
mainfrom
bbusa/zkboost-mock-proving-time-defaults

Conversation

@barnabasbusa
Copy link
Copy Markdown
Collaborator

Summary

  • A partial override like mock_proving_time: { kind: constant } silently became 0ms because dict.get on the missing ms key returned 0, while omitting mock_proving_time entirely fell back to the launcher's hardcoded 6000ms — surprising and inconsistent.
  • Fill in kind-appropriate defaults during input validation so partial overrides behave like the no-override case. Defaults now scale with slot_duration_ms (2/3 of slot): constant.ms, random.min_ms/max_ms (default_ms/2 .. default_ms*2), and linear.ms_per_mgas.
  • Updated the launcher's defensive fallback and the auto-injected default zkvm to use the same 2/3-of-slot scaling.

Test plan

  • kurtosis lint --format . passes
  • zkvms: [{ kind: mock, proof_type: reth-zisk }] → mock proving time defaults to 4000ms on 6s slots (2/3 of 6000)
  • zkvms: [{ kind: mock, proof_type: reth-zisk, mock_proving_time: { kind: constant } }] → resolves to 4000ms (no longer 0ms)
  • zkvms: [{ kind: mock, proof_type: reth-zisk, mock_proving_time: { kind: random } }] → resolves to 2000–8000ms on 6s slots
  • On a 12s slot, defaults scale to 8000ms / 4000–16000ms

Partial overrides like `mock_proving_time: { kind: constant }` silently
became 0ms because dict.get on the missing `ms` key returned 0. Fill in
kind-appropriate defaults (scaled to 2/3 of slot_duration_ms) during
input validation so partial configs behave like the no-override case.
The launcher fallback and the auto-injected default zkvm scale with
slot duration too.
@qu0b-reviewer
Copy link
Copy Markdown

qu0b-reviewer Bot commented May 19, 2026

🤖 qu0b-reviewer

Summary

The PR fills in missing mock_proving_time keys with slot-duration-scaled defaults (2/3 of slot), and adds a linear kind handler. The logic is sound for constant and random, but the linear default has a semantic mismatch.

Issues

  • 🟡 concern src/package_io/input_parser.star:611ms_per_mgas defaults to default_ms (e.g. 4000ms for 6s slots, 8000ms for 12s), but ms_per_mgas is "milliseconds per mega-gas" — a rate, not a duration. The README example shows 150 ms_per_mgas. Feeding a value 26–53× larger than the documented example will produce proofs far slower than intended, silently. There is no validation for an upper bound. If this is intentional to keep defaults consistent across kinds, that rationale should be documented.

Suggestions

  • src/package_io/input_parser.star:578 — New linear block intentionally leaves MockProvingTimeLinearMsPerMgas at 0 when mock_proving_time is not provided (the get defaults to 0). But launch_zkboost now uses the correct slot-scaled default for the outer .get() call. This path works correctly — no action needed.

Reviewed @ 6d90f0c5
"Cheaper to fly to Vietnam than to fix it here."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant