fix: commitments [skip-line-limit]

[0xjei]

see #1184

Summary by CodeRabbit

• Refactor

  • Public keys and secret components now use per-modulus (multi-polynomial) representation.
  • Commitment handling unified into a domain-separated, wrapper-driven API; challenge and verification flows updated to use it.
  • Per-modulus range and consistency checks added for clearer validation.

• New Features

  • Centralized commitment helpers exposed for reuse.
  • Ciphertext commitment generation switched to the new commitment API.

• Chores

  • On-chain verifier key constant updated.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Review	Updated (UTC)
crisp	Skipped		Jan 23, 2026 2:12pm
enclave-docs	Skipped		Jan 23, 2026 2:12pm

[coderabbitai]

📝 Walkthrough

Walkthrough

Changed single-polynomial flows to per-modulus arrays ([Polynomial; L]), introduced a domain-separated commitments API (compute_commitments and wrappers), and updated callers across TRBFV PK, share verification, BFV enc/dec, Greco, examples, and SDK to use per-modulus commitment/challenge functions.

Changes

Cohort / File(s)	Summary
Main executables circuits/bin/insecure/pk_trbfv/src/main.nr, circuits/bin/production/pk_trbfv/src/main.nr	main parameter e_sm: Polynomial<N> → e_sm: [Polynomial<N>; L]; updated TrbfvPublicKey::new calls.
TRBFV public key circuits/lib/src/core/trbfv_pk.nr	TrbfvPublicKey.e_sm changed to [Polynomial<N>; L]; constructor and verify updated to per-modulus commitments, per-modulus range checks, and new challenge API.
Verification & shares circuits/lib/src/core/trbfv_verify_shares.nr, circuits/lib/src/core/trbfv_dec_share.nr	Replaced payload-based commitment flows with direct per-modulus commitment calls (e.g., compute_secret_*, compute_spm_commitment_from_shares, compute_aggregated_shares_commitment::<N,L,...>); challenge calls use generic wrappers.
BFV encryption & decryption circuits/lib/src/core/bfv_enc.nr, circuits/lib/src/core/bfv_dec.nr, circuits/lib/src/core/bfv_pk.nr	Removed prepare_* payload helpers; callers now use direct commitment/challenge functions (compute_pk_bfv_commitment, compute_spm_commitment_from_message, compute_bfv_enc_challenge, etc.).
Aggregation, Greco & PK aggregation circuits/lib/src/core/trbfv_pk_agg.nr, circuits/lib/src/core/greco.nr	Removed intermediate payload builders; replaced with per-modulus commitment/aggregation APIs (compute_pk_trbfv_commitment, compute_pk_agg_commitment) and updated challenge invocation signatures.
Commitments library circuits/lib/src/math/commitments.nr	Major redesign: added DS_* domain separators, compute_commitments wrapper, payload builders (single_polynomial_payload, multiple_polynomial_payload), many new commitment/challenge wrappers (e.g., compute_pk_trbfv_commitment, compute_secret_e_sm_commitment, compute_spm_commitment_from_message, compute_aggregated_shares_commitment, compute_dec_share_challenge), and removed legacy prepare_* payload functions.
BFV PK circuits/lib/src/core/bfv_pk.nr	Dropped intermediate payload; now directly calls compute_pk_bfv_commitment(self.pk0, self.pk1).
Greco circuits/lib/src/core/greco.nr	Removed commitment_payload helper; generate_challenge updated to new commitment API (compute_greco_challenge_commitment generic call).
Examples / CRISP circuits examples/CRISP/circuits/src/ciphertext_addition.nr, examples/CRISP/circuits/src/commitments.nr, examples/CRISP/circuits/src/main.nr	Replaced sponge/hash-based challenge and commitment helpers with domain-separated compute_commitments and compute_ct_commitment; removed old hash module and switched callers to new commitments APIs.
CRISP SDK / zk-inputs examples/CRISP/crates/zk-inputs/src/commitments.rs, .../ciphertext_addition.rs, .../lib.rs	Added Rust-side compute_commitment API (Arc), replaced inline poly-commitment logic, updated call signatures to use shared BFV params and new commitment function.
Verifier contract examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol	Updated VK_HASH constant value.
Scripts scripts/check-license-headers.sh	Exit code now non-zero when missing files are detected (exit 1 if MISSING_FILES present).

Sequence Diagram(s)

sequenceDiagram
    participant Main as rgba(10,90,200,0.5) Main
    participant PK as rgba(20,160,60,0.5) TrbfvPublicKey
    participant Comm as rgba(200,100,20,0.5) Commitments
    participant Verifier as rgba(150,40,180,0.5) Verifier

    Main->>PK: TrbfvPublicKey::new(..., e_sm_array, ...)
    PK->>Comm: compute_secret_sk_commitment(sk)
    PK->>Comm: compute_secret_e_sm_commitment(e_sm_array)
    PK->>Comm: compute_pk_trbfv_commitment(pk0, pk1)
    Verifier->>Comm: compute_pk_trbfv_challenge::<L>(inputs)
    Verifier->>Comm: compute_aggregated_shares_commitment::<N,L,BIT>(aggregated_shares)
    Verifier->>PK: verify commitments & per-modulus range checks

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• refactor: pvss migration [skip-line-limit] #1170 — similar TRBFV public-key change to per-modulus e_sm and related call-site updates.
• feat: add pk_commitment for greco #1083 — related Greco commitment/challenge and payload handling changes.
• feat: greco bound optimization #823 — overlapping core API edits around polynomial flattening and range/bit-width interfaces.

Suggested labels

noir

Suggested reviewers

• cedoor
• zahrajavar
• ctrlc03

Poem

🐰 I hopped through polynomials, lining each in L,
Domain tags stitched softly into every spell.
Secrets split by modulus, commitments hum true,
A rabbit’s tiny circuit—fresh, modular, new.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'fix: commitments [skip-line-limit]' is vague and generic. It uses a non-descriptive term 'commitments' that doesn't convey the scope or nature of the specific changes made.	Replace with a more specific title that describes the main change, e.g., 'fix: refactor commitment API to use domain separators' or 'fix: consolidate commitment functions with unified wrapper API'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@circuits/lib/src/core/trbfv_verify_shares.nr`:
- Line 1: Remove the stray top-of-file documentation artifact '///
crates/generator/commitments' from trbfv_verify_shares.nr; simply delete that
line so the file no longer begins with the misplaced comment and the module
starts with the intended code content.

🧹 Nitpick comments (2)

circuits/lib/src/core/trbfv_verify_shares.nr (1)

8-12: LGTM!

The import of prepare_l_polynomial_commitment_payload on a separate line is functionally correct. Consider consolidating it with the existing import block for cleaner organization.

♻️ Optional consolidation

 use crate::math::commitments::{
     compute_secret_commitment, compute_shares_party_modulus_commitment,
-    prepare_shares_party_modulus_commitment_payload, prepare_single_polynomial_commitment_payload,
+    prepare_l_polynomial_commitment_payload, prepare_shares_party_modulus_commitment_payload,
+    prepare_single_polynomial_commitment_payload,
 };
-use crate::math::commitments::prepare_l_polynomial_commitment_payload;

circuits/lib/src/math/commitments.nr (1)

57-63: Clarify the helper’s purpose as “L‑polynomial” payload.

The current doc still reads like the old aggregated-shares helper; a small rename improves intent.

✏️ Suggested doc tweak

-/// Prepares the payload for aggregated shares commitment.
-/// Used in C1, C2, C4.
+/// Prepares the payload for L-polynomial commitment (L polynomials).
+/// Used in C1, C2, C4.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@circuits/lib/src/core/trbfv_verify_shares.nr`:
- Around line 175-179: The docstring is wrong: compute_secret_e_sm_commitment
(which calls multiple_polynomial_payload::<N, L, BIT_E_SM>) hashes all L modulus
polynomials, not just the first modulus; update the docstring near
compute_secret_e_sm_commitment/compute_secret_e_sm_commitment::<N, L, BIT_E_SM>
to state that the commitment is computed over all L RNS polynomials (matching
multiple_polynomial_payload's behavior), or alternatively change the
implementation to only hash the first modulus — but prefer the docstring change
so the assertion comparing compute_secret_e_sm_commitment(self.secret_e_sm) to
self.expected_secret_commitment correctly documents that all L polynomials are
included.

[0xjei]

we need gnosisguild/zkfhe-generator#72 merged and check the CI again (I think the greco commitment is breaking things now)

[cedoor]

After merging gnosisguild/zkfhe-generator#72 and updating the zkfhe-generator crates, the CRISP ZK verifier also needs to be updated.

[0xjei]

nnnamo