Skip to content

test(stm): split the IVC prover monolith into separated tests - #3505

Merged
hjeljeli32 merged 5 commits into
mainfrom
hjeljeli32/3468-replace-prove-all-scenarios-monolith
Sep 1, 2026
Merged

test(stm): split the IVC prover monolith into separated tests#3505
hjeljeli32 merged 5 commits into
mainfrom
hjeljeli32/3468-replace-prove-all-scenarios-monolith

Conversation

@hjeljeli32

@hjeljeli32 hjeljeli32 commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Content

prove_all_scenarios ran eight scenarios in one test to amortize a single setup. It costs 1393.9 s on the nightly and, with the two keygen-only prover_setup tests of the single-threaded ivc-snark-sequential group, forms an 1830 s serial tail, 59 % of the job's 3116 s test step.

Splitting it naively costs more than it saves: at user / real = 7.39 the test already saturates the runner's four vCPUs and peaks at 11.02 GiB against 16 GB, so the group must stay serial and each extra process adds 70-125 s. The split is paid for by removing proving instead — the five rejection scenarios generate no proofs, and the six prove_with_transcript calls cover only four distinct (transition type, transcript) pairs. Warm and local: 175.5 s to 126.0 s, −28 %. On CI, normalised against the two untouched keygen-only tests of the same group, the two replacements cost 951 s against the monolith's 1264 s (−313 s, −25 %) taking the serial tail to 1408 s and the job's test step to 2348 s.

Changes

  • Rejection scenarios to the fast tier (prover_input.rs, proof.rs): four run_rejects_* helpers become prepare tests asserting typed IvcCircuitError variants instead of is_err(), at ~0.36 s on every PR rather than nightly only, each failed inside prepare, which since Recover the ignored IVC prover input preparation tests #3466 needs no SRS, keygen or cache. The fifth duplicated prepare_rejects_invalid_snark_proof and is deleted.
  • Monolith split, mature next-epoch proving removed (proof.rs): prove_bootstrap_produces_first_epoch_proof_and_rolling_state (3 proofs) and prove_same_epoch_produces_proof_without_rolling_state (1 proof) share a plain build_slow_test_context(), no OnceLock, which amortizes nothing under nextest's process-per-test model. run_next_epoch_path adds no new pair, and prepare_at_next_epoch_carries_lookahead_protocol_parameters already pins its state and accumulator against the same asset.
  • Context decoupled from asset generation (proof.rs): the slow context builds Global and the genesis bootstrap input from the committed genesis fixture rather than AssetGenerationSetup. No measurable wall-time change — setup spans 7.1-8.7 s for identical artifacts and the ~1.0 GB proving key deserialize dominates — so it is kept for the decoupling, not for speed.
  • Unchanged: .config/nextest.toml and filter-slow-tests.sh match on module path, so the renamed tests stay grouped and watched. The coverage the reduction rests on still passes; restore a mature next-epoch proving case if production later couples transition type with accumulator provenance, public inputs or transcript selection.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • All check jobs of the CI have succeeded
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • No new TODOs introduced

Issue(s)

Closes #3468

@hjeljeli32 hjeljeli32 self-assigned this Aug 30, 2026
@hjeljeli32 hjeljeli32 added the run-slow-tests Technical label to run slow tests tiers in the CI. label Aug 30, 2026
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown

Test Results

     5 files  ± 0     221 suites  ±0   1h 57m 34s ⏱️ + 1h 1m 50s
 3 567 tests +62   3 567 ✅ +62  0 💤 ±0  0 ❌ ±0 
11 683 runs  +71  11 683 ✅ +71  0 💤 ±0  0 ❌ ±0 

Results for commit 3de9bf0. ± Comparison against base commit 69572a0.

♻️ This comment has been updated with latest results.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restructures the IVC prover tests to reduce CI runtime by splitting the former “monolith” scenario test into focused cases and moving rejection-path coverage to faster prepare-only tests, while decoupling slow-test setup from asset-generation machinery.

Changes:

  • Added fast IvcProverInput::prepare rejection tests that assert typed IvcCircuitError variants (tampered message, mismatched AVK, corrupted previous IVC proof, mismatched Global).
  • Replaced the prove_all_scenarios monolith with two standalone slow prover tests (bootstrap and same-epoch), using a shared slow-context builder.
  • Built slow-test Global and genesis bootstrap input from committed genesis fixtures and verification-context assets (not asset-generation setup).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
mithril-stm/src/proof_system/ivc_halo2_snark/prover_input.rs Adds fast-tier prepare rejection tests using committed assets and typed error assertions.
mithril-stm/src/proof_system/ivc_halo2_snark/proof.rs Splits the slow prover monolith into two tests and rebuilds slow context from committed fixtures/assets.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread mithril-stm/src/proof_system/ivc_halo2_snark/proof.rs

@jpraynaud jpraynaud left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@damrobi damrobi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@hjeljeli32
hjeljeli32 force-pushed the hjeljeli32/3468-replace-prove-all-scenarios-monolith branch from d26bca9 to 3de9bf0 Compare August 31, 2026 22:59
@hjeljeli32
hjeljeli32 deployed to testing-2-preview September 1, 2026 00:00 — with GitHub Actions Active
@hjeljeli32
hjeljeli32 deployed to testing-preview September 1, 2026 00:00 — with GitHub Actions Active
@hjeljeli32
hjeljeli32 merged commit 60ef0ff into main Sep 1, 2026
51 checks passed
@hjeljeli32
hjeljeli32 deleted the hjeljeli32/3468-replace-prove-all-scenarios-monolith branch September 1, 2026 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-slow-tests Technical label to run slow tests tiers in the CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace prove_all_scenarios monolith with separated tests

4 participants