Skip to content

feat(stm): add recursive IVC circuit benchmarks#3432

Open
hjeljeli32 wants to merge 11 commits into
mainfrom
hjeljeli32/3412-ivc-recursive-circuit-benchmarks
Open

feat(stm): add recursive IVC circuit benchmarks#3432
hjeljeli32 wants to merge 11 commits into
mainfrom
hjeljeli32/3412-ivc-recursive-circuit-benchmarks

Conversation

@hjeljeli32

@hjeljeli32 hjeljeli32 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Content

This PR adds benchmarks for the recursive IVC circuit's prover, verifier, off-circuit accumulator fold, and setup. They are driven end-to-end by a thin benchmark-internals-gated façade that delegates to the production IVC proof-system code, so the measured operations are the same code paths run in production. The inner circuit is the small committed (degree-13) non-recursive certificate, sourced from the committed golden assets plus one additive genesis fixture, and every transition path (genesis, same-epoch, next-epoch) is covered.

Each run builds the shared recursive setup once (a full keygen) and reports single-observation timings in consolidated tables. Given a degree-19 recursive proof costs tens of seconds and the keygen runs for minutes, Criterion's sampling is not used (mirroring the existing halo2_prover_modes bench).

Changes

  • Benchmark harness: new mithril-stm/benches/ivc_halo2_snark.rs (gated behind future_snark and
    benchmark-internals), a single-observation report driven by the IvcBenchEnv façade. One shared keygen
    per run; a per-path prove/verify/fold table and a setup cold-vs-warm table.
  • Asset layer / fixtures: moved the read-only IVC asset layer out of the test tree (embedded_assets)
    so benchmarks and tests can share it, and added one additive genesis benchmark fixture. Existing golden
    assets are unchanged.
  • Measured operations: recursive prover (create_proof, Poseidon and Blake2b transcripts); verification
    as both the isolated KZG opening and the full verification (opening plus the folded-accumulator check); the
    off-circuit accumulator fold per transition path (genesis is an accumulator passthrough, so it has no fold);
    and setup cold vs warm, measured separately for the SRS and for the certificate + recursive circuit keys.
  • Façade: added a warm-start SRS load through the production cached read path with the same downsize as
    production setup, symmetric cold_start/warm_start naming, and a measure_keys that mirrors production
    setup (certificate verifying key then recursive key pair) so cold and warm cover the same key layers.
  • CLI: a fail-closed argument parser (in a benchmark-internals-gated bench_cli module, unit-tested) —
    --list to print the ids, an optional single literal id/prefix filter, and rejection of anything that
    could otherwise trigger an accidental multi-minute keygen (e.g. value-taking options, --test,
    --profile-time, regex filters).

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 #3412

Comment thread mithril-stm/src/circuits/halo2_ivc/bench_helpers.rs Fixed
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown

Test Results

     5 files  ± 0     209 suites  ±0   1h 49m 38s ⏱️ - 1h 16m 34s
 3 345 tests  - 41   3 345 ✅  - 41  0 💤 ±0  0 ❌ ±0 
11 080 runs   - 41  11 080 ✅  - 41  0 💤 ±0  0 ❌ ±0 

Results for commit 8d57eda. ± Comparison against base commit 936eadd.

This pull request removes 51 and adds 10 tests. Note that renamed tests count towards both.
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::index_out_of_bounds
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::index_too_large_for_circuit_range
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::indices_not_increasing
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::leaf_merkle_path_mismatch
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::leaf_swap_keep_merkle_path
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::leaf_wrong_verification_key
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::merkle_path_corrupt_sibling
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::merkle_path_flip_position
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::merkle_path_length_long
mithril-stm ‑ circuits::halo2::tests::golden::cases::negative::slow::merkle_path_length_short
…
mithril-stm ‑ circuits::halo2_ivc::bench_cli::tests::a_single_literal_filter_is_kept
mithril-stm ‑ circuits::halo2_ivc::bench_cli::tests::control_flags_map_to_their_variants
mithril-stm ‑ circuits::halo2_ivc::bench_cli::tests::help_and_version_take_precedence_over_control_flags
mithril-stm ‑ circuits::halo2_ivc::bench_cli::tests::no_args_runs_everything
mithril-stm ‑ circuits::halo2_ivc::bench_cli::tests::tolerated_valueless_flags_do_not_disturb_parsing
mithril-stm ‑ circuits::halo2_ivc::bench_cli::tests::unsupported_and_malformed_inputs_are_rejected
mithril-stm ‑ circuits::halo2_ivc::bench_cli::tests::value_taking_options_are_rejected_not_swallowed
mithril-stm ‑ circuits::halo2_ivc::tests::golden::positive::genesis_benchmark_fixture_is_deterministic_and_valid
mithril-stm ‑ protocol::key_registration::register::tests::register_by_entry_rejects_same_snark_key_with_different_concatenation_key
mithril-stm ‑ protocol::key_registration::register::tests::register_by_entry_rejects_same_verification_key_with_different_stake

♻️ 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 introduces an end-to-end benchmark suite for the recursive IVC Halo2 SNARK in mithril-stm, driven through a benchmark-internals-gated façade that delegates to the production proof-system paths. It also refactors the IVC golden-asset layer so both tests and benchmarks can share the same embedded fixtures, and adds a deterministic genesis benchmark fixture plus a fail-closed benchmark CLI filter.

Changes:

  • Add a new ivc_halo2_snark benchmark target with manual single-observation timing tables and a fail-closed CLI filter.
  • Move the read-only embedded golden-asset decoding into a shared embedded_assets module and add an additive genesis benchmark fixture (plus a test to guard determinism/validity).
  • Expose a small set of internal APIs under benchmark-internals to reuse production setup/prove/verify/fold code paths in benchmarks.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
mithril-stm/src/proof_system/ivc_halo2_snark/verifier_setup.rs Extends test-only verifier-setup constructor gating to benchmarks.
mithril-stm/src/proof_system/ivc_halo2_snark/proof.rs Makes transcript-based proving helper accessible to the benchmark façade.
mithril-stm/src/proof_system/ivc_halo2_snark/mod.rs Re-exports IvcProverInput for benchmarks/tests.
mithril-stm/src/circuits/trusted_setup.rs Exposes unsafe-SRS helpers to benchmarks (previously tests only).
mithril-stm/src/circuits/halo2_ivc/types.rs Allows test helper conversion (from_field) for benchmarks.
mithril-stm/src/circuits/halo2_ivc/tests/golden/positive.rs Adds a determinism/validity guard test for the new genesis benchmark fixture.
mithril-stm/src/circuits/halo2_ivc/tests/common/generators/setup.rs Adds filesystem path for the additive genesis benchmark fixture asset.
mithril-stm/src/circuits/halo2_ivc/tests/common/generators/asset_generation.rs Adds generator + ignored entrypoint to produce the benchmark fixture asset.
mithril-stm/src/circuits/halo2_ivc/tests/common/field_encoding.rs Re-exports field-encoding helper from the shared embedded asset layer.
mithril-stm/src/circuits/halo2_ivc/tests/common/asset_readers.rs Keeps test-only asset writers; re-exports shared embedded loaders/types.
mithril-stm/src/circuits/halo2_ivc/mod.rs Wires in benchmark-only modules and the shared embedded asset module.
mithril-stm/src/circuits/halo2_ivc/embedded_assets.rs New shared read-only embedded asset layer (types + decoders + include_bytes!).
mithril-stm/src/circuits/halo2_ivc/bench_helpers.rs New benchmark façade building setup/fixtures and delegating to production code paths.
mithril-stm/src/circuits/halo2_ivc/bench_cli.rs New fail-closed CLI parser (unit-tested) for benchmark filtering.
mithril-stm/Cargo.toml Registers the new ivc_halo2_snark benchmark with required features.
mithril-stm/benches/ivc_halo2_snark.rs New benchmark harness producing consolidated timing tables (manual timing).
Comments suppressed due to low confidence (1)

mithril-stm/src/circuits/trusted_setup.rs:212

  • TrustedSetupProvider::with_unsafe_srs now compiles under feature = "benchmark-internals", but it still uses multiple unwrap() calls for serialization and filesystem IO. This can panic during benchmark runs (e.g. permission / disk full / temp dir issues) and also violates the project’s “no unwrap outside tests” guideline. Consider returning StmResult<Self> and propagating errors with ? (and Context) instead of panicking.
        let mut srs_bytes = Vec::new();
        srs.write_custom(&mut srs_bytes, SerdeFormat::RawBytes).unwrap();

        let srs_dir = base_dir.join(MITHRIL_CIRCUIT_SRS_FOLDER);
        create_dir_all(&srs_dir).unwrap();

Comment thread mithril-stm/src/proof_system/ivc_halo2_snark/verifier_setup.rs
Comment thread mithril-stm/src/circuits/halo2_ivc/mod.rs
Comment thread mithril-stm/src/circuits/halo2_ivc/embedded_assets.rs
Comment thread mithril-stm/src/proof_system/ivc_halo2_snark/verifier_setup.rs
@hjeljeli32
hjeljeli32 deployed to testing-2-preview July 24, 2026 00:37 — with GitHub Actions Active
@hjeljeli32
hjeljeli32 deployed to testing-preview July 24, 2026 00:37 — with GitHub Actions Active

@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 👍

pub(crate) mod rolling_state;
pub(crate) mod verifier_setup;

#[cfg(any(test, feature = "benchmark-internals"))]

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.

That's probably enough:

Suggested change
#[cfg(any(test, feature = "benchmark-internals"))]
#[cfg(feature = "benchmark-internals")]

Comment on lines +83 to +96
pub struct PreparedStep {
path: TransitionPath,
// Recursive prove inputs (item 5): `create_proof` over prebuilt circuit data.
circuit_data: IvcCircuitData,
public_inputs: Vec<CircuitBase>,
// Verification inputs (item 6): the committed step proof.
proof_bytes: IvcProofBytes,
proof_state: State,
proof_accumulator: Accumulator<BlstrsEmulation>,
message: Vec<u8>,
// Accumulator-fold inputs (item 7): two already-collapsed accumulators + the rolling one.
// `None` for genesis (passthrough — no certificate or previous-IVC accumulator).
fold: Option<FoldInputs>,
}

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.

What are exactly items 5, 6 and 7?


/// Builds a same-epoch or next-epoch fixture from the committed recursive chain state and the
/// matching step output asset, mirroring the production `IvcProverInput::prepare` flow.
fn prepare_certificate_step(&self, path: TransitionPath) -> StmResult<PreparedStep> {

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.

This function is too long and should be splitted for readability and maintainability.

Some(accumulator)
}

// --- Setup cold/warm measurements (item 8) ---

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.

This can be removed.

}

/// Shared benchmark environment delegating to the production IVC proof-system code.
pub struct IvcBenchEnv {

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.

It's usually a convention to put the definition of the struct before its implementation in a module.

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.

These bench_ prefixed modules should at least live in a dedicated module, and these module should probably be in the benches folder.

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.

Also the benches folder would greatly benefit from a README file explaining that there is a CLI that can be run with the benchmarks and details its usage.

@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 👍

Comment on lines +104 to +106
use sha2::{Digest, Sha256};

use crate::signature_scheme::BaseFieldElement;

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.

I think we can move those out of the function

/// `--list`: print ids and return (no environment build).
List,
/// `--ignored`: nothing to run (no benches are ignored).
Ignored,

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.

Why do we have an Ignored option if no benches are ignored? Is it in anticipation of potential future ignored benches?

/// is the circuit keys: derived (empty key cache) vs loaded (populated key cache). This is
/// therefore `full_setup/cold_keys` vs `full_setup/cached_keys` — not a warm-SRS measurement.
/// The bench harness labels the two groups (`cold_keys` vs `cached_keys`) accordingly.
pub fn measure_full_setup(cache_dir: &Path) -> StmResult<()> {

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.

Claude flagged this function as not used anywhere, maybe it can be removed?

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.

Add benchmarks for the IVC SNARK recursive circuit

5 participants