test(stm): consolidate the recursive circuit MockProver tests - #3486
Open
hjeljeli32 wants to merge 10 commits into
Open
test(stm): consolidate the recursive circuit MockProver tests#3486hjeljeli32 wants to merge 10 commits into
hjeljeli32 wants to merge 10 commits into
Conversation
hjeljeli32
requested review from
curiecrypt,
damrobi and
jpraynaud
as code owners
August 24, 2026 03:09
hjeljeli32
temporarily deployed
to
testing-preview
August 24, 2026 04:20 — with
GitHub Actions
Inactive
hjeljeli32
temporarily deployed
to
testing-2-preview
August 24, 2026 04:20 — with
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
Consolidates recursive-circuit MockProver tests while strengthening rejection assertions with exact failure signatures.
Changes:
- Adds public-input layout and failure-signature helpers.
- Introduces satisfiable non-genesis fixtures and positive checks.
- Consolidates negative transition, encoding, and accumulator cases.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
transitions/positive.rs |
Adds non-genesis acceptance tests. |
transitions/negative/same_epoch.rs |
Consolidates same-epoch binding checks. |
transitions/negative/next_epoch.rs |
Consolidates next-epoch binding checks. |
transitions/negative/genesis.rs |
Consolidates genesis public-input checks. |
transitions/mod.rs |
Clarifies transition-test scope. |
in_circuit/state_transition.rs |
Removes redundant negative tests. |
in_circuit/public_inputs.rs |
Removes migrated MockProver checks. |
in_circuit/mod.rs |
Updates test organization documentation. |
in_circuit/accumulator.rs |
Checks the complete accumulator signature. |
golden/positive.rs |
Uses explicit genesis fixtures. |
encoding/negative.rs |
Adds exact preimage failure signatures. |
common/public_input_layout.rs |
Defines named public-input rows. |
common/mod.rs |
Exposes new test helpers. |
common/helpers.rs |
Adds asset-backed step fixtures. |
common/generators/mod.rs |
Removes unused generator exports. |
common/failure_signature.rs |
Adds exact MockProver failure classification. |
Suppressed comments (1)
mithril-stm/src/circuits/halo2_ivc/tests/common/public_input_layout.rs:58
- These crate-visible enum variants are missing doc comments. Document each variant so the public row-layout API remains self-describing.
pub(crate) enum StateField {
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
damrobi
approved these changes
Aug 24, 2026
damrobi
left a comment
Collaborator
There was a problem hiding this comment.
LGTM 👍
Just one small nitpick and the Copilot comments that seem relevant
hjeljeli32
force-pushed
the
hjeljeli32/3464-consolidate-recursive-circuit-mockprover-tests
branch
from
August 25, 2026 01:07
fe8639f to
9f019ec
Compare
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
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.
Content
This PR consolidates the MockProver tests in
circuits::halo2_ivcand replaces every negative case'sverify().is_err()with an exact failure signature. Consolidating test functions saves nothing, sinceMockProver::runre-synthesizes on every call, so only removing invocations helps. The tests were also weaker than their names: six were found to reject even with their tampering removed, because the cheap stimulus pairs empty proof slots with a trivial accumulator, which balances only at genesis where the gating scales both accumulator contributions to the group identity.Measured locally at
-j 4on theci-tests-snarkprofile: the slow tier goes from 403.2 s to 277.9 s aggregated and 107.0 s to 76.5 s wall, 22 tests becoming 15 and 20 MockProver invocations becoming 14. Net of −6.5 s drift on untouched tests, −118.7 s is attributable; about 65 s of the saving went on four deliberately added invocations. Fast tier unchanged at 541 passing. Per-item numbers are in #3464.Changes
tests/common/failure_signature.rs): a runner generic overCircuit<NativeField>asserting that the failing rows on the public-statement instance column are exactly the expected set and that every failure is a permutation failure. Unit-tested on a minimal two-instance-column circuit.tests/common/public_input_layout.rs): named row accessors for the global, state and accumulator sections, so no test carries a row literal, guarded by section-length and serialization-order tests.tests/common/helpers.rs,tests/transitions/positive.rs): same-epoch and next-epoch stimuli built from the committed step assets, so nothing is proved at test time, plus two untampered acceptance cases; the first in-circuit positive coverage of non-genesis steps.tests/transitions/negative/): thirteen single-field cases become three, one per transition context. Coverage widens at no cost, the step counter and the two genesis verification-key rows having been untested.tests/encoding/negative.rs): the three byte-range cases now assert the message row plus the decoded field their window feeds, flipping one byte rather than filling a range. A new case isolates the message equality.tests/in_circuit/accumulator.rs): an untampered canary on a clone, then the whole accumulator section asserted rather than one element. Keeps its freshly proved certificate, the only unseen proof the circuit verifies.row + 1, since two tampered cells in one permutation class stay equal under a shared delta and left one accumulator row unreported.in_circuit/state_transition.rsin full andpublic_inputs.rs's slow module: six cases, three vacuous and three duplicating the genesis case. Both weak rejection helpers are gone.main: the chain-link constraints have no negative coverage, which the deleted cases did not provide either, and an exact row set does not prove each row carries the field named for it. Both are recorded for follow-up.Pre-submit checklist
Issue(s)
Closes #3464