refactor(stm): move the IVC circuit onto the Relation trait

[hjeljeli32]

Content

The IVC circuit implements Halo2's low level Circuit trait with a hand written configuration and its own column pool, while the certificate circuit implements the Midnight standard library's Relation. The two configure their chips differently, serialize their verifying keys in different formats, and the IVC configuration duplicates what ZkStdLibArch already provides. The IVC circuit now implements Relation, declares its chips through ZkStdLibArch, takes its verifier gadget from the standard library, and shares the MidnightVK and MidnightPK key formats.

This is a breaking circuit key change. The standard library registers gates and selectors in a different order, so the recursive verifying key's commitments change although its shape does not — still degree 19, still 45 fixed commitments — and every asset derived from it was regenerated. The degree was measured against production certificate metadata before any key was generated and holds at 19; lowering nr_pow2range_cols below 4 was checked as the runbook requires, and 1 to 3 columns all require degree 20.

Changes

• Relation implementation (circuit.rs, constraint_builder.rs, witness_assignments.rs, schnorr_signature.rs): the synthesis body moves to Relation::circuit, IvcCircuit carries the metadata fixing the constraint system and IvcCircuitData becomes its witness. The builder borrows the standard library's chips instead of owning its own.
• Standard library configuration (circuit.rs, config.rs deleted): one recursive_circuit_architecture() feeds both the declared chips and the key generation metadata, so they cannot drift. The hand written configuration, its column pool and the errors guarding it are removed.
• Shared key format (key_serialization.rs added at circuits/, halo2_ivc/key_serialization.rs deleted, keys.rs): one MidnightVK codec serves both circuits, the recursive newtypes wrap the Midnight types, and key generation goes through zk::setup_vk and zk::setup_pk. The inner verifying key is byte identical, so this adds a 23 byte envelope without altering circuit identity.
• Recursive decoder guards (keys.rs, errors.rs): both circuits now encode keys identically, so only the declared architecture distinguishes them. The decoders check the architecture and every declared degree before the bytes reach the dependency's readers, require a standalone encoding to be fully consumed, and check the fixed commitment count against the configured constraint system. Serde uses the same guard, which is the path the verifier data takes.
• Recursive cache identity (key_provider.rs): non production entries were fingerprinted from the certificate key alone and are trusted without comparison, so a stale recursive entry would have been loaded against the new constraint system. The recursive fingerprint now includes the recursive key; the certificate fingerprint is unchanged and pinned by a test.
• Regenerated assets (recursive_circuit_verification_key_for_production.bin, eight files under tests/assets/, tests/assets/README.md): both recursive verifying keys, the verification context, the three step outputs, the chain state and both accumulator anchors, plus the two recursive key digests, the combiner challenge and the ancillary encoding digest. The README now lists those source constants, which no documentation covered.

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
  • Update README file (if relevant)
  • No new TODOs introduced

Issue(s)

Closes #3428

[github-actions]

Test Results

     5 files  ± 0     221 suites  ±0   1h 35m 59s ⏱️ + 36m 13s
 3 692 tests +65   3 692 ✅ +65  0 💤 ±0  0 ❌ ±0 
11 836 runs  +71  11 836 ✅ +71  0 💤 ±0  0 ❌ ±0 

Results for commit ecb9b63. ± Comparison against base commit f7f05f6.

♻️ This comment has been updated with latest results.

[Copilot]

🟡 Changes recommended

Unresolved key architecture/trailing-byte validation and witness-copy findings remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Refactors the Halo2 IVC circuit onto Midnight’s Relation trait, standardizes configuration and key formats, and regenerates dependent assets. This is a breaking circuit-key change.

Changes:

• Replaces handwritten configuration with ZkStdLibArch.
• Shares Midnight key serialization and adds decoder/cache guards.
• Regenerates recursive keys, proofs, digests, and fixtures.

File summaries

File	Reviewed change / final comment
mithril-stm/src/protocol/aggregate_signature/ancillary_data.rs	Updates ancillary encoding digest golden.
mithril-stm/src/proof_system/halo2_ivc_snark/verifier_setup.rs	Adds recursive key envelope validation coverage.
mithril-stm/src/proof_system/halo2_ivc_snark/proof.rs	Uses relation-based IVC proving.
mithril-stm/src/circuits/verification_key_digest.rs	Updates verification-key digest goldens.
mithril-stm/src/circuits/trusted_setup.rs	Defines the trusted SRS degree.
mithril-stm/src/circuits/mod.rs	Registers shared serialization.
mithril-stm/src/circuits/key_serialization.rs	Adds shared VK codecs. Critical (3 votes): enforce certificate-architecture validation in the non-recursive wrapper and serde path. Moderate (1 vote): reject trailing bytes in the standalone decoder.
mithril-stm/src/circuits/key_provider.rs	Updates recursive cache identity.
mithril-stm/src/circuits/halo2/keys.rs	Uses shared VK serialization.
mithril-stm/src/circuits/halo2/key_serialization.rs	Retains certificate proving-key serialization.
mithril-stm/src/circuits/halo2_ivc/witness_assignments.rs	Separates circuit metadata inputs.
mithril-stm/src/circuits/halo2_ivc/tests/verification_key_computation.rs	Updates recursive VK computation.
mithril-stm/src/circuits/halo2_ivc/tests/transitions/positive.rs	Supplies the recursive relation.
mithril-stm/src/circuits/halo2_ivc/tests/transitions/negative/same_epoch.rs	Updates negative test setup.
mithril-stm/src/circuits/halo2_ivc/tests/transitions/negative/next_epoch.rs	Updates negative test setup.
mithril-stm/src/circuits/halo2_ivc/tests/transitions/negative/genesis.rs	Updates negative test setup.
mithril-stm/src/circuits/halo2_ivc/tests/off_circuit/circuit_validation.rs	Moves degree validation to IvcCircuit.
mithril-stm/src/circuits/halo2_ivc/tests/in_circuit/genesis_gating.rs	Updates witness construction.
mithril-stm/src/circuits/halo2_ivc/tests/in_circuit/accumulator.rs	Updates accumulator tests.
mithril-stm/src/circuits/halo2_ivc/tests/golden/positive.rs	Updates golden proving setup.
mithril-stm/src/circuits/halo2_ivc/tests/encoding/positive.rs	Updates VK decoding tests.
mithril-stm/src/circuits/halo2_ivc/tests/encoding/negative.rs	Updates rejection tests.
mithril-stm/src/circuits/halo2_ivc/tests/common/helpers.rs	Adds relation-aware helpers.
mithril-stm/src/circuits/halo2_ivc/tests/common/generators/verification_key.rs	Generates relation-based VKs.
mithril-stm/src/circuits/halo2_ivc/tests/common/generators/setup.rs	Updates relation-based key generation.
mithril-stm/src/circuits/halo2_ivc/tests/common/generators/proofs.rs	Proves through MidnightCircuit.
mithril-stm/src/circuits/halo2_ivc/tests/common/generators/asset_generation.rs	Regenerates relation-based assets.
mithril-stm/src/circuits/halo2_ivc/tests/common/failure_signature.rs	Updates mock-prover wrappers.
mithril-stm/src/circuits/halo2_ivc/tests/common/asset_readers.rs	Reads and writes Midnight VKs.
mithril-stm/src/circuits/halo2_ivc/tests/assets/README.md	Documents regenerated constants.
mithril-stm/src/circuits/halo2_ivc/mod.rs	Updates exports and removes handwritten configuration.
mithril-stm/src/circuits/halo2_ivc/midnight_backend.rs	Simplifies backend aliases.
mithril-stm/src/circuits/halo2_ivc/keys.rs	Adds Midnight key wrappers and guards. Nit (2 votes): add a valid MidnightPK<IvcCircuit> round-trip test. Moderate (1 vote): reject trailing bytes after MidnightPK::read.
mithril-stm/src/circuits/halo2_ivc/key_serialization.rs	Removes the obsolete raw recursive key codec.
mithril-stm/src/circuits/halo2_ivc/gadgets/schnorr_signature.rs	Uses standard-library gadgets.
mithril-stm/src/circuits/halo2_ivc/errors.rs	Adds key validation errors.
mithril-stm/src/circuits/halo2_ivc/embedded_assets.rs	Loads enveloped recursive VKs.
mithril-stm/src/circuits/halo2_ivc/constraint_builder.rs	Borrows standard-library chips.
mithril-stm/src/circuits/halo2_ivc/config.rs	Removes handwritten configuration.
mithril-stm/src/circuits/halo2_ivc/circuit.rs	Implements Relation and separates witness data. Moderate (1 vote): avoid repeatedly cloning the full witness payload during synthesis.
mithril-stm/src/circuits/halo2_ivc/bench/helpers.rs	Carries the relation through benchmarks.

Review details

Suppressed comments (3)

mithril-stm/src/circuits/halo2_ivc/circuit.rs:141

• Each witness.clone() clones the entire IvcCircuitData before extracting one field. That payload contains both proof byte vectors and the accumulator, so synthesis now performs several deep copies per proof/MockProver run; the old circuit stored each Value separately and did not copy the whole payload. Split the witness fields without repeatedly cloning the aggregate data, especially since this path runs for both recursive proof transcripts.

        let global_value = witness.clone().map(|data| data.global);
        let state_value = witness.clone().map(|data| data.state);
        let witness_value = witness.clone().map(|data| data.witness);
        let certificate_proof_value = witness.clone().map(|data| data.certificate_proof);
        let ivc_proof_value = witness.clone().map(|data| data.ivc_proof);

mithril-stm/src/circuits/halo2_ivc/keys.rs:272

• The recursive proving-key reader does not check that MidnightPK::read consumed the standalone input. Unlike the verifying-key decoder above, KeyProvider::cache_state can therefore accept a valid proving key followed by arbitrary bytes; add the same trailing-byte check before returning the cached key.

impl TryFromBytes for RecursiveCircuitProvingKey {
    fn try_from_bytes(bytes: &[u8]) -> StmResult<Self> {
        RecursiveCircuitVerifyingKey::validate_encoded_proving_key_header(bytes)?;

        let mut reader = bytes;
        Ok(Self(
            MidnightPK::<IvcCircuit>::read(&mut reader, KEY_SERDE_FORMAT)
                .with_context(|| "Failed to deserialize the recursive proving key")?,
        ))

mithril-stm/src/circuits/key_serialization.rs:33

• The standalone byte decoder returns after MidnightVK::read without checking whether reader was fully consumed. The certificate wrapper uses this implementation, so a certificate key field with appended bytes is silently accepted and canonical re-serialization no longer represents the input; enforce full consumption for this standalone codec (while keeping a separate streaming path for assets).

• Files reviewed: 41/50 changed files
• Comments generated: 2
• Review effort level: Lite

---

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

[jpraynaud]

LGTM 👍

[damrobi]

LGTM 👍
Some imports are out of order or inside functions so maybe you can rearrange them

[kitounliu]

witness.clone() copies the whole witness 5 times. A simpler way could be using the reference,

let data = witness.as_ref();
let global_value = data.map(|d| d.global.clone());
let state_value = data.map(|d| d.state.clone());
let witness_value = data.map(|d| d.witness.clone());
let certificate_proof_value = data.map(|d| d.certificate_proof.clone());
let ivc_proof_value = data.map(|d| d.ivc_proof.clone());
let accumulator_value = data.map(|d| d.accumulator.clone());

[kitounliu]

LGTM