refactor: pvss migration [skip-line-limit]#1170
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
❌ License Header Check Failed Some files are missing the required SPDX license header. Please add the following header to the beginning of all You can run Or run |
Contributor
📝 WalkthroughWalkthroughThis pull request restructures the Noir circuits library from a monolithic layout into organized insecure and production variants, consolidates cryptographic circuit implementations into a unified library with generalized configurations, introduces new BFV and TRBFV circuit implementations with parameterized verification flows, and updates CI workflows to use wrapper scripts instead of direct commands. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
cedoor
changed the title
cedoor
force-pushed
the
refactor/pvss-migration
branch
from
9b860e2 to
c73afc8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 Fix all issues with AI agents
In `@circuits/bin/aggregation/fold/Nargo.toml`:
- Line 9: The dependency entry for bb_proof_verification in Nargo.toml is pinned
to a nightly tag; update the tag value from v3.0.0-nightly.20260102 to a stable
release candidate (e.g., v3.0.0-rc.6) to avoid instability, keeping the git URL
and directory unchanged and ensuring any CI or lockfiles are refreshed/committed
after the change; locate the bb_proof_verification key in Nargo.toml to make
this edit.
In `@circuits/bin/aggregation/production/verify_shares_trbfv_wrapper/Nargo.toml`:
- Line 9: The dependency bb_proof_verification in Nargo.toml is pinned to a
nightly tag; change it to a stable release by replacing the tag value
"v3.0.0-nightly.20260102" with the stable release tag "v0.63.0" (i.e., update
the bb_proof_verification entry so it references tag = "v0.63.0") to ensure
reproducible, stable builds for production; keep the git and directory fields
the same unless a different release location is required.
In `@circuits/bin/aggregation/production/verify_shares_trbfv_wrapper/src/main.nr`:
- Around line 21-23: The loop currently ignores the boolean return value of
verify_honk_proof_non_zk; capture its result into a variable (e.g., let ok =
verify_honk_proof_non_zk(...)) and assert or handle failures immediately (for
example call assert(ok) or return an error/panic) so invalid proofs can't pass;
update the for loop that calls verify_honk_proof_non_zk and ensure any failure
short-circuits verification with a clear error path.
In `@circuits/bin/insecure/enc_bfv_e_sm/src/main.nr`:
- Around line 14-31: The two parameters expected_pk_commitment and
expected_message_commitment are declared as private but used as public inputs;
update the main function signature to mark these parameters as public (prepend
pub) so they're bound as public inputs during proof verification—locate the
main(...) signature where expected_pk_commitment: Field and
expected_message_commitment: Field are listed and change their declarations to
pub expected_pk_commitment: Field and pub expected_message_commitment: Field
accordingly.
In `@circuits/bin/insecure/pk_agg_trbfv/README.md`:
- Line 1: Update the README title to use lowercase to match other circuit
READMEs: change "Threshold BFV Public Key Aggregation circuit" to "threshold bfv
public key aggregation circuit" (also update any occurrences inside the file),
and ensure formatting matches other files like "enc_bfv smudging noise circuit"
for consistent documentation naming conventions.
In `@circuits/bin/insecure/pk_trbfv/README.md`:
- Line 1: The README title uses inconsistent capitalization ("Threshold BFV
Public Key circuit"); change it to match the project's lowercase style by
replacing that string with "threshold bfv public key circuit" (update the README
entry in circuits/bin/insecure/pk_trbfv/README.md and any occurrences of the
exact phrase "Threshold BFV Public Key circuit" within that file to the
lowercase form).
In `@circuits/bin/production/dec_share_agg_trbfv/Nargo.toml`:
- Line 2: The package name in Nargo.toml is dec_shares_agg_trbfv but the
workspace member is dec_share_agg_trbfv; make them identical by either renaming
the package to dec_share_agg_trbfv in Nargo.toml or updating the workspace
member to dec_shares_agg_trbfv in the workspace manifest so the crate name and
workspace entry match (ensure you update any references/imports that use
dec_share(s)_agg_trbfv accordingly).
In `@circuits/bin/production/verify_shares_trbfv_sk/README.md`:
- Line 1: Capitalize the first word of the README title and replace the
single-line note with an expanded README for the production "verify_shares_trbfv
secret key circuit (PVSS `#2a`)" that includes: a brief Purpose section describing
the circuit's role, Usage instructions showing how to instantiate/run the
production circuit, Input/Output specifications listing expected types/formats
and example values, a short Example invocation snippet, and Security
Considerations outlining assumptions/threats and safe deployment notes; update
the README.md content accordingly using the circuit name "verify_shares_trbfv
secret key circuit (PVSS `#2a`)" to locate the context.
In `@circuits/lib/src/configs/insecure/trbfv.nr`:
- Around line 98-141: The K1 and R1 bound constants are inverted causing invalid
ranges: swap GRECO_K1_LOW_BOUND and GRECO_K1_UP_BOUND so GRECO_K1_LOW_BOUND = 4
and GRECO_K1_UP_BOUND = 5, and swap the entries in
GRECO_R1_LOW_BOUNDS/GRECO_R1_UP_BOUNDS so the first pair becomes
GRECO_R1_LOW_BOUNDS = [260, 258] and GRECO_R1_UP_BOUNDS = [261, 258], then
ensure GRECO_CONFIGS continues to reference these corrected constants.
In `@circuits/lib/src/core/trbfv_dec_share.nr`:
- Around line 174-215: Add a range-check for the decryption share vector d
inside check_range_bounds (before it is flattened/hashed into the transcript):
use the existing Configs.decryption_share_bound and call the same
range_check_2bounds helper (e.g.
d.range_check_2bounds::<BIT_D>(self.configs.decryption_share_bound[basis_idx],
self.configs.decryption_share_bound[basis_idx])) for each CRT basis index so the
decryption_share_bound is actually enforced prior to challenge generation.
In `@circuits/lib/src/core/trbfv_dec_shares_agg.nr`:
- Around line 228-270: In verify_decoding: Q is built as a Field (q_modulus)
which can overflow and t_times_u_bn_q is never reduced mod Q; compute Q as a
big-number using Enclave_TRBFV_8192::from by accumulating self.configs.qis into
q_bn (replace q_modulus accumulation), then after computing t_bn_q * u_bn_q
explicitly reduce t_times_u_bn_q = (t_bn_q * u_bn_q).umod(q_bn) before any
centering logic; use that reduced t_times_u_bn_q for the q_half_bn comparison,
subtraction (q_bn - t_times_u_bn_q) and subsequent umod(t_bn_q) calls so
centered reduction is correct.
- Around line 111-158: The code assumes self.party_ids is strictly increasing
when computing Lagrange signs; add an assertion early in the Lagrange
coefficient routine to enforce that property (e.g., assert each
self.party_ids[i] < self.party_ids[i+1]) so denom_sign_negative logic
(num_greater = T - party_idx) is valid. Place the check before computing
diffs/denominator in the method using self.party_ids (the loop that builds diffs
and computes numerator_sign_negative / denom_sign_negative) and fail-fast with a
clear error message referencing party_ids to prevent silent incorrect
coefficients.
In `@circuits/lib/src/math/helpers.nr`:
- Around line 297-302: The determinism test currently only checks that
digests2.get(0) != 0; update it to assert equality between the previously
computed digest and the new one from compute_safe to ensure deterministic
output. Locate the two calls to compute_safe (the original digest variable and
digests2) and replace/add assertions to verify lengths match (digests.len() ==
digests2.len()) and that corresponding elements are equal (e.g.,
assert(digests.get(i) == digests2.get(i)) or simply assert(digests.get(0) ==
digests2.get(0))) for the domain_separator, elements, io_pattern case.
In `@circuits/lib/src/math/modulo/U128.nr`:
- Around line 145-156: The div_mod implementation must ensure rhs is actually
invertible; instead of relying on the weak equality check after calling unsafe
__div_mod, call self.inv_mod(rhs) to assert invertibility and obtain rhs_inv,
then compute result = self.mul_mod(self.reduce_mod(lhs), rhs_inv) (or otherwise
replace the unsafe __div_mod usage with multiplication by rhs_inv) and adjust
the post-check to verify self.mul_mod(result, rhs) == self.reduce_mod(lhs).
Reference functions: div_mod, inv_mod, __div_mod, mul_mod, reduce_mod.
In `@circuits/lib/src/math/polynomial.nr`:
- Around line 123-142: In eval_mod, the lazy-reduction comparison currently
casts values to u64 which loses correctness for moduli >= 2^64; change the
comparisons to use u128 instead: use m = q.get_mod_field() as u128 (or treat m
as u128) and compare acc as u128 (e.g., acc as u128 >= m) in both the loop and
the final reduction, keeping the calls to q.reduce_mod(acc) unchanged; update
references to acc, m, eval_mod, ModU128, get_mod_field, and reduce_mod
accordingly so reductions trigger for large moduli.
In `@scripts/lint-circuits.sh`:
- Around line 27-33: The loop currently reads null-separated paths into pkg_dir
but uses find ... -print0 | xargs -0 dirname which converts to newline-separated
output causing the read -d '' to break; change the pipeline to feed the found
files directly into the while loop (e.g. find ... -print0 | while IFS= read -r
-d '' file; do pkg_dir="$(dirname "$file")";
prover_file="${pkg_dir}/Prover.toml"; ... done) so dirname is computed inside
the loop and you can safely use read -d '' with files variable names (pkg_dir,
prover_file, created_files) without xargs.
In `@scripts/test-circuits.sh`:
- Around line 4-5: The test script currently changes into circuits/lib and runs
`nargo test --workspace`, but `circuits/lib/Nargo.toml` is a library without a
[workspace] so the binary circuit packages are skipped; fix by either adding a
[workspace] section to `circuits/lib/Nargo.toml` that lists or globs the binary
members under circuits/bin (e.g., include the 32 aggregation/insecure/production
members) or change `scripts/test-circuits.sh` to run tests across all packages
(for example, run `nargo test` from the repo root or iterate `circuits/bin/*`
and run `nargo test` per package) so that `pvss_lib` and all binary circuits are
tested.
♻️ Duplicate comments (1)
circuits/bin/production/verify_shares_trbfv_sk/Nargo.toml (1)
1-9: Duplicate package name concern already flagged.This manifest shares the same package name concern already noted in the insecure variant review.
🧹 Nitpick comments (18)
circuits/bin/insecure/enc_bfv_sk/README.md (1)
1-1: Expand the README to provide essential context.This minimal README would benefit from additional documentation explaining:
- What makes this instantiation insecure and why it exists
- The meaning of "PVSS
#3a"- Appropriate use cases (e.g., testing/development only)
- Key differences from the production version
Clear documentation is especially important for circuits marked as "insecure" to prevent misuse.
📝 Example expanded README
# BFV Secret Key Encryption Circuit (Insecure) Insecure instantiation of the BFV secret key encryption circuit (PVSS step 3a). ## ⚠️ Warning This is an **insecure** variant intended for **testing and development only**. Do NOT use in production environments. ## Purpose This circuit implements [brief description of what the circuit does]. ## Insecurity This variant uses [explain what makes it insecure - e.g., smaller parameters, simplified operations, disabled security checks, etc.]. ## PVSS Reference Part of PVSS protocol step 3a: [brief explanation or link to specification]. ## Usage Refer to `circuits/bin/production/enc_bfv_sk` for the production-ready version.scripts/lint-circuits.sh (1)
26-32: Unusedcreated_filesarray; cleanup deletes all Prover.toml files.The
created_filesarray tracks which files were created but is never used. Line 48 unconditionally deletes allProver.tomlfiles, including any that existed before the script ran. This is inconsistent—either use the tracked array for targeted cleanup, or remove the tracking as dead code.♻️ Option A: Use tracked files for cleanup (preserves pre-existing files)
- # Remove all Prover.toml files (they're generated by nargo and in .gitignore) - find . -name "Prover.toml" -type f -delete + # Remove only the Prover.toml files we created + for f in "${created_files[@]}"; do + rm -f "$f" + done♻️ Option B: Remove unused tracking (if deleting all is intentional)
# Find all package directories and create empty Prover.toml to prevent nargo from creating them - created_files=() while IFS= read -r -d '' nargo_file; do pkg_dir=$(dirname "$nargo_file") prover_file="${pkg_dir}/Prover.toml" if [ ! -f "$prover_file" ]; then touch "$prover_file" - created_files+=("$prover_file") fi done < <(find . -name "Nargo.toml" -type f -print0 2>/dev/null)Also applies to: 47-48
circuits/lib/Nargo.toml (1)
8-10: Git tags forkeccak256(v0.1.1) andbignum(v0.0.2) are valid. Consider pinning to immutable commit hashes if the dependency system supports it, for improved reproducibility.circuits/bin/insecure/dec_bfv_sk/README.md (1)
1-1: Consider expanding the README with a proper heading and brief usage context.The current single-line description is minimal. Adding a Markdown heading (
# dec_bfv_sk) and a brief explanation of the circuit's purpose, inputs/outputs, and why it's marked "insecure" would improve discoverability and maintainability.📝 Suggested README structure
-insecure instantiation of dec_bfv secret key circuit (PVSS `#4a`) +# dec_bfv_sk + +Insecure instantiation of the BFV decryption secret key verification circuit (PVSS `#4a`). + +> ⚠️ **Warning**: This circuit uses insecure parameters and is intended for testing/development only. + +## Overview + +This circuit verifies BFV decryption shares against expected commitments.circuits/lib/src/configs/production/bfv.nr (1)
50-105: Reduce duplicated constants to avoid drift.
ENC_BFV_TandENC_BFV_Q_MOD_TduplicatePLAINTEXT_MODULUSandQ_MOD_T. Reusing the globals makes future updates safer.♻️ Proposed change
-pub global ENC_BFV_T: Field = 18014398509481984; -pub global ENC_BFV_Q_MOD_T: Field = 1082658244788225; +pub global ENC_BFV_T: Field = PLAINTEXT_MODULUS; +pub global ENC_BFV_Q_MOD_T: Field = Q_MOD_T;circuits/bin/insecure/greco/README.md (1)
1-1: Consider expanding the README with a short warning.
A brief heading plus “not for production” helps avoid misuse.✍️ Suggested text
-insecure instantiation of Greco circuit +# Greco (insecure) + +This circuit is intended for insecure/testing usage only. Do not use in production.circuits/bin/production/verify_shares_trbfv_e_sm/README.md (1)
1-1: Consider expanding documentation for production circuits.The README provides minimal information about the circuit. Consider adding:
- Circuit purpose and behavior
- Input/output parameters
- Usage examples or integration notes
- Related circuits in the PVSS workflow
circuits/bin/production/pk_bfv/README.md (1)
1-1: Consider expanding documentation for production circuits.Same suggestion as other production READMEs: consider adding circuit purpose, parameters, usage examples, and workflow context.
circuits/bin/production/greco/README.md (1)
1-1: Minor inconsistency in documentation format.Other circuit READMEs in this PR include a PVSS number reference (e.g.,
(PVSS#5)). Consider adding one here for consistency if Greco is part of the PVSS workflow, or this is fine if Greco is a separate component.circuits/bin/production/dec_bfv_sk/Nargo.toml (1)
1-9: LGTM!Manifest structure is correct, and the relative path to the lib dependency is accurate for the directory layout.
Minor nit: there's an extra blank line (line 7) that could be removed for consistency.
🧹 Optional cleanup
[package] name = "dec_bfv_sk" type = "bin" authors = ["Gnosis Guild / Enclave"] version = "1.0.0-beta.15" - [dependencies] lib = { path = "../../../lib" }circuits/bin/insecure/dec_share_agg_trbfv/Nargo.toml (1)
1-9: Naming inconsistency: directory vs package name.The directory is named
dec_share_agg_trbfv(singular "share"), but the package name isdec_shares_agg_trbfv(plural "shares"). This mismatch is reflected in the workspace configuration, which references the package by its directory name (dec_share_agg_trbfv), while the package manifest uses the plural form. Additionally, the codebase semantics—including theDecryptionSharesAggregationclass and constants likeDEC_SHARES_AGG_CONFIGS—suggest the plural form is semantically correct.Align the naming by either renaming the directory to
dec_shares_agg_trbfv(and updating workspace members) or renaming the package todec_share_agg_trbfvto match the directory.circuits/bin/insecure/verify_shares_trbfv_sk/README.md (1)
1-1: Consider adding a markdown header for consistency.The content accurately describes the circuit's purpose. A minor improvement would be to format it as a proper markdown heading.
📝 Suggested formatting
-insecure instantiation of verify_shares_trbfv secret key circuit (PVSS `#2a`) +# verify_shares_trbfv_sk (insecure) + +Insecure instantiation of verify_shares_trbfv secret key circuit (PVSS `#2a`).circuits/bin/production/verify_shares_trbfv_e_sm/Nargo.toml (1)
1-9: LGTM!The manifest is correctly configured with the appropriate package metadata and dependency path. Minor nit: there's an extra blank line between the
[package]section and[dependencies](line 7), which could be removed for consistency with other manifests.circuits/bin/production/enc_bfv_sk/Nargo.toml (1)
1-9: LGTM!The manifest is correctly configured for the BFV encryption binary with proper dependency path to the lib crate. Same minor nit as the previous file: extra blank line at line 7.
circuits/bin/production/verify_shares_trbfv_e_sm/src/main.nr (1)
18-33: Remember to include public outputs in verifier inputs.This entrypoint exposes a public output array; downstream verifiers (e.g., Solidity) must include all public outputs alongside public inputs when forming the verification array.
Based on learnings, ensure the verifier input array concatenates public inputs and public outputs for this circuit.
circuits/bin/production/dec_share_trbfv/src/main.nr (1)
14-17: Unused constantsN_PARTIESandT.These constants are declared but not referenced anywhere in the circuit. If they're intended as documentation for the expected threshold scheme parameters, consider adding a comment clarifying this. Otherwise, they could be removed to avoid confusion.
circuits/bin/insecure/dec_share_trbfv/src/main.nr (1)
14-17: Unused constantsN_PARTIESandT.Same as the production variant, these constants are declared but not used in the circuit logic. Consider documenting their purpose or removing them.
circuits/bin/insecure/verify_shares_trbfv_sk/src/main.nr (1)
13-16: Avoid config drift forN_PARTIES/T.These type-level constants must stay aligned with
VERIFY_SHARES_CONFIGS_SK. If the config module exposes them, consider reusing those constants (or add a compile-time/assert check) to avoid silent divergence.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (124)
circuits/Nargo.tomlcircuits/bin/aggregation/Nargo.tomlcircuits/bin/aggregation/fold/Nargo.tomlcircuits/bin/aggregation/fold/src/main.nrcircuits/bin/aggregation/insecure/verify_shares_trbfv_wrapper/Nargo.tomlcircuits/bin/aggregation/insecure/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/aggregation/pk_trbfv_wrapper/Nargo.tomlcircuits/bin/aggregation/pk_trbfv_wrapper/src/main.nrcircuits/bin/aggregation/production/verify_shares_trbfv_wrapper/Nargo.tomlcircuits/bin/aggregation/production/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/insecure/Nargo.tomlcircuits/bin/insecure/dec_bfv_e_sm/Nargo.tomlcircuits/bin/insecure/dec_bfv_e_sm/README.mdcircuits/bin/insecure/dec_bfv_e_sm/src/main.nrcircuits/bin/insecure/dec_bfv_sk/Nargo.tomlcircuits/bin/insecure/dec_bfv_sk/README.mdcircuits/bin/insecure/dec_bfv_sk/src/main.nrcircuits/bin/insecure/dec_share_agg_trbfv/Nargo.tomlcircuits/bin/insecure/dec_share_agg_trbfv/README.mdcircuits/bin/insecure/dec_share_agg_trbfv/src/main.nrcircuits/bin/insecure/dec_share_trbfv/Nargo.tomlcircuits/bin/insecure/dec_share_trbfv/README.mdcircuits/bin/insecure/dec_share_trbfv/src/main.nrcircuits/bin/insecure/enc_bfv_e_sm/Nargo.tomlcircuits/bin/insecure/enc_bfv_e_sm/README.mdcircuits/bin/insecure/enc_bfv_e_sm/src/main.nrcircuits/bin/insecure/enc_bfv_sk/Nargo.tomlcircuits/bin/insecure/enc_bfv_sk/README.mdcircuits/bin/insecure/enc_bfv_sk/src/main.nrcircuits/bin/insecure/greco/Nargo.tomlcircuits/bin/insecure/greco/README.mdcircuits/bin/insecure/greco/src/main.nrcircuits/bin/insecure/pk_agg_trbfv/Nargo.tomlcircuits/bin/insecure/pk_agg_trbfv/README.mdcircuits/bin/insecure/pk_agg_trbfv/src/main.nrcircuits/bin/insecure/pk_bfv/Nargo.tomlcircuits/bin/insecure/pk_bfv/README.mdcircuits/bin/insecure/pk_bfv/src/main.nrcircuits/bin/insecure/pk_trbfv/Nargo.tomlcircuits/bin/insecure/pk_trbfv/README.mdcircuits/bin/insecure/pk_trbfv/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_e_sm/Nargo.tomlcircuits/bin/insecure/verify_shares_trbfv_e_sm/README.mdcircuits/bin/insecure/verify_shares_trbfv_e_sm/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_sk/Nargo.tomlcircuits/bin/insecure/verify_shares_trbfv_sk/README.mdcircuits/bin/insecure/verify_shares_trbfv_sk/src/main.nrcircuits/bin/production/Nargo.tomlcircuits/bin/production/dec_bfv_e_sm/Nargo.tomlcircuits/bin/production/dec_bfv_e_sm/README.mdcircuits/bin/production/dec_bfv_e_sm/src/main.nrcircuits/bin/production/dec_bfv_sk/Nargo.tomlcircuits/bin/production/dec_bfv_sk/README.mdcircuits/bin/production/dec_bfv_sk/src/main.nrcircuits/bin/production/dec_share_agg_trbfv/Nargo.tomlcircuits/bin/production/dec_share_agg_trbfv/README.mdcircuits/bin/production/dec_share_agg_trbfv/src/main.nrcircuits/bin/production/dec_share_trbfv/Nargo.tomlcircuits/bin/production/dec_share_trbfv/README.mdcircuits/bin/production/dec_share_trbfv/src/main.nrcircuits/bin/production/enc_bfv_e_sm/Nargo.tomlcircuits/bin/production/enc_bfv_e_sm/README.mdcircuits/bin/production/enc_bfv_e_sm/src/main.nrcircuits/bin/production/enc_bfv_sk/Nargo.tomlcircuits/bin/production/enc_bfv_sk/README.mdcircuits/bin/production/enc_bfv_sk/src/main.nrcircuits/bin/production/greco/Nargo.tomlcircuits/bin/production/greco/README.mdcircuits/bin/production/greco/src/main.nrcircuits/bin/production/pk_agg_trbfv/Nargo.tomlcircuits/bin/production/pk_agg_trbfv/README.mdcircuits/bin/production/pk_agg_trbfv/src/main.nrcircuits/bin/production/pk_bfv/Nargo.tomlcircuits/bin/production/pk_bfv/README.mdcircuits/bin/production/pk_bfv/src/main.nrcircuits/bin/production/pk_trbfv/Nargo.tomlcircuits/bin/production/pk_trbfv/README.mdcircuits/bin/production/pk_trbfv/src/main.nrcircuits/bin/production/verify_shares_trbfv_e_sm/Nargo.tomlcircuits/bin/production/verify_shares_trbfv_e_sm/README.mdcircuits/bin/production/verify_shares_trbfv_e_sm/src/main.nrcircuits/bin/production/verify_shares_trbfv_sk/Nargo.tomlcircuits/bin/production/verify_shares_trbfv_sk/README.mdcircuits/bin/production/verify_shares_trbfv_sk/src/main.nrcircuits/crates/libs/greco/Nargo.tomlcircuits/crates/libs/greco/README.mdcircuits/crates/libs/polynomial/Nargo.tomlcircuits/crates/libs/polynomial/README.mdcircuits/crates/libs/safe/README.mdcircuits/lib/Nargo.tomlcircuits/lib/src/configs/insecure/bfv.nrcircuits/lib/src/configs/insecure/mod.nrcircuits/lib/src/configs/insecure/trbfv.nrcircuits/lib/src/configs/mod.nrcircuits/lib/src/configs/production/bfv.nrcircuits/lib/src/configs/production/mod.nrcircuits/lib/src/configs/production/trbfv.nrcircuits/lib/src/core/bfv_dec.nrcircuits/lib/src/core/bfv_enc.nrcircuits/lib/src/core/bfv_pk.nrcircuits/lib/src/core/greco.nrcircuits/lib/src/core/mod.nrcircuits/lib/src/core/trbfv_dec_share.nrcircuits/lib/src/core/trbfv_dec_shares_agg.nrcircuits/lib/src/core/trbfv_pk.nrcircuits/lib/src/core/trbfv_pk_agg.nrcircuits/lib/src/core/trbfv_verify_shares.nrcircuits/lib/src/lib.nrcircuits/lib/src/math/commitments.nrcircuits/lib/src/math/helpers.nrcircuits/lib/src/math/mod.nrcircuits/lib/src/math/modulo/U128.nrcircuits/lib/src/math/modulo/mod.nrcircuits/lib/src/math/modulo/unconstrained_U128.nrcircuits/lib/src/math/polynomial.nrcircuits/lib/src/math/safe.nrexamples/CRISP/circuits/Nargo.tomlexamples/CRISP/circuits/src/ciphertext_addition.nrexamples/CRISP/circuits/src/hash.nrexamples/CRISP/circuits/src/main.nrexamples/CRISP/circuits/src/utils.nrexamples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.solscripts/lint-circuits.shscripts/test-circuits.sh
💤 Files with no reviewable changes (6)
- circuits/Nargo.toml
- circuits/crates/libs/greco/README.md
- circuits/crates/libs/polynomial/README.md
- circuits/crates/libs/safe/README.md
- circuits/crates/libs/polynomial/Nargo.toml
- circuits/crates/libs/greco/Nargo.toml
🧰 Additional context used
🧠 Learnings (22)
📓 Common learnings
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: packages/circuits/crates/libs/polynomial/src/lib.nr:121-133
Timestamp: 2025-08-27T13:53:47.832Z
Learning: In Noir circuits, prefer relying on built-in array bounds checking rather than adding explicit preconditions for buffer overflow protection, as the language already provides safety guarantees that will fail the circuit on out-of-bounds access.
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 1142
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:141-147
Timestamp: 2025-12-23T17:18:32.343Z
Learning: When verifying Noir circuit proofs in Solidity, the public inputs array passed to the verifier must include both the circuit's public inputs AND public outputs. For example, if a Noir circuit has 3 public inputs and returns 2064 public output fields (-> pub [Field; 2064]), the verification array must contain all 2067 values (3 + 2064).
📚 Learning: 2025-11-08T01:31:47.505Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 969
File: packages/enclave-sdk/src/enclave-sdk.ts:101-103
Timestamp: 2025-11-08T01:31:47.505Z
Learning: In packages/enclave-sdk/src/enclave-sdk.ts, the TRBFV protocol parameter setup is intentionally incomplete as of PR `#969`. The constructor sets BFV_THRESHOLD parameters for TRBFV, but the encryption methods (encryptNumber, encryptVector, encryptNumberAndGenInputs, encryptVectorAndGenInputs) intentionally only support BFV and will throw "Protocol not supported" for TRBFV until future implementation is completed.
Applied to files:
circuits/bin/insecure/pk_agg_trbfv/README.mdcircuits/bin/insecure/dec_bfv_sk/README.mdcircuits/bin/insecure/pk_bfv/README.mdcircuits/bin/insecure/pk_trbfv/README.mdcircuits/bin/insecure/enc_bfv_sk/src/main.nrcircuits/bin/insecure/dec_share_trbfv/README.mdcircuits/bin/insecure/enc_bfv_sk/Nargo.tomlcircuits/bin/insecure/enc_bfv_sk/README.mdcircuits/lib/src/core/bfv_enc.nrcircuits/bin/insecure/enc_bfv_e_sm/README.mdcircuits/lib/src/core/trbfv_pk.nrcircuits/lib/src/configs/production/bfv.nrcircuits/bin/production/enc_bfv_sk/Nargo.tomlcircuits/lib/src/core/trbfv_dec_share.nrcircuits/lib/src/configs/insecure/trbfv.nrcircuits/lib/src/configs/insecure/bfv.nrcircuits/lib/src/configs/production/trbfv.nr
📚 Learning: 2025-10-04T14:00:17.916Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 660
File: crates/events/src/enclave_event/decryptionshare_created.rs:23-26
Timestamp: 2025-10-04T14:00:17.916Z
Learning: In the enclave codebase using threshold BFV cryptography, decryption shares (as in DecryptionshareCreated events) are not sensitive data and can be safely logged or displayed, as individual shares reveal nothing without reaching the threshold number of shares.
Applied to files:
circuits/bin/insecure/verify_shares_trbfv_e_sm/README.mdcircuits/bin/production/dec_share_agg_trbfv/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_sk/README.mdcircuits/bin/production/dec_share_trbfv/README.mdcircuits/bin/insecure/dec_share_agg_trbfv/README.mdcircuits/bin/insecure/dec_bfv_sk/README.mdcircuits/bin/production/verify_shares_trbfv_sk/README.mdcircuits/bin/insecure/dec_share_agg_trbfv/src/main.nrcircuits/bin/production/dec_share_trbfv/src/main.nrcircuits/bin/insecure/dec_share_trbfv/README.mdcircuits/lib/src/core/trbfv_dec_shares_agg.nrcircuits/bin/insecure/dec_share_trbfv/src/main.nrcircuits/lib/src/core/trbfv_dec_share.nr
📚 Learning: 2024-09-26T03:11:29.311Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-09-26T03:11:29.311Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Applied to files:
examples/CRISP/circuits/Nargo.tomlcircuits/lib/src/core/mod.nrcircuits/lib/src/math/mod.nrexamples/CRISP/circuits/src/hash.nrcircuits/lib/src/math/commitments.nrexamples/CRISP/circuits/src/utils.nrcircuits/lib/Nargo.toml
📚 Learning: 2024-10-22T02:36:01.448Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/data/Cargo.toml:0-0
Timestamp: 2024-10-22T02:36:01.448Z
Learning: In `packages/ciphernode/data/Cargo.toml`, `actix-rt` is only used in tests and should remain in `[dev-dependencies]`.
Applied to files:
examples/CRISP/circuits/Nargo.tomlcircuits/bin/insecure/pk_bfv/Nargo.tomlcircuits/bin/insecure/pk_trbfv/Nargo.tomlcircuits/bin/production/pk_trbfv/Nargo.tomlcircuits/bin/insecure/pk_agg_trbfv/Nargo.toml
📚 Learning: 2025-08-25T10:28:56.174Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 657
File: Cargo.toml:32-34
Timestamp: 2025-08-25T10:28:56.174Z
Learning: The examples/CRISP directory has its own Cargo.toml workspace configuration with members like "server", "wasm-crypto", "program/core", "program/client", etc. The root workspace intentionally excludes "examples/CRISP/server", "examples/CRISP/program", and "examples/CRISP/wasm-crypto" to prevent double workspace membership, which is the correct approach for self-contained example workspaces.
Applied to files:
examples/CRISP/circuits/Nargo.tomlcircuits/bin/insecure/Nargo.tomlcircuits/bin/aggregation/Nargo.tomlcircuits/bin/production/Nargo.toml
📚 Learning: 2025-09-22T15:08:29.814Z
Learnt from: ozgurarmanc
Repo: gnosisguild/enclave PR: 734
File: packages/circuits/crates/libs/polynomial/src/lib.nr:140-155
Timestamp: 2025-09-22T15:08:29.814Z
Learning: Greco (packages/circuits/crates/libs/greco/src/lib.nr) performs range_check_1bound/2bounds on all polynomials (u, e0/e1, k1, pk*, r*, p*) before serialization; packer/flatten rely on these bounds, so per-limb asserts inside packer are unnecessary in this crate’s flow.
Applied to files:
examples/CRISP/circuits/Nargo.tomlcircuits/bin/production/greco/src/main.nrexamples/CRISP/circuits/src/ciphertext_addition.nrexamples/CRISP/circuits/src/hash.nrcircuits/lib/src/math/polynomial.nrcircuits/bin/insecure/greco/src/main.nrcircuits/lib/src/core/bfv_enc.nrcircuits/lib/src/core/greco.nrcircuits/lib/src/math/helpers.nrcircuits/lib/src/core/trbfv_pk.nrexamples/CRISP/circuits/src/utils.nrexamples/CRISP/circuits/src/main.nrcircuits/lib/src/configs/insecure/trbfv.nr
📚 Learning: 2025-05-07T15:18:20.056Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 388
File: packages/commons/src/bfv/mod.rs:38-48
Timestamp: 2025-05-07T15:18:20.056Z
Learning: The `build_bfv_params` and related functions in the commons BFV utilities intentionally use panic for error handling as a temporary solution. The team plans to refactor these to use proper Result-based error handling in the future.
Applied to files:
circuits/lib/src/configs/production/mod.nrcircuits/lib/src/core/mod.nrcircuits/bin/insecure/enc_bfv_e_sm/src/main.nrcircuits/lib/src/configs/insecure/mod.nr
📚 Learning: 2024-10-22T02:10:34.864Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:82-83
Timestamp: 2024-10-22T02:10:34.864Z
Learning: In the file `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs`, when reviewing test functions like `generate_pk_share`, minor performance optimizations (e.g., minimizing mutex locks) are not a priority.
Applied to files:
circuits/bin/production/dec_share_agg_trbfv/src/main.nrcircuits/bin/production/enc_bfv_e_sm/src/main.nrcircuits/bin/insecure/dec_share_agg_trbfv/Nargo.tomlcircuits/lib/src/core/mod.nrcircuits/bin/aggregation/production/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/insecure/dec_share_agg_trbfv/src/main.nrcircuits/bin/insecure/dec_bfv_e_sm/src/main.nrcircuits/bin/production/dec_share_trbfv/src/main.nrcircuits/bin/production/greco/src/main.nrcircuits/bin/insecure/dec_bfv_sk/src/main.nrcircuits/bin/production/enc_bfv_sk/src/main.nrcircuits/bin/insecure/pk_agg_trbfv/src/main.nrcircuits/bin/production/dec_bfv_sk/src/main.nrexamples/CRISP/circuits/src/ciphertext_addition.nrcircuits/bin/production/dec_bfv_e_sm/src/main.nrcircuits/bin/insecure/enc_bfv_sk/src/main.nrcircuits/bin/insecure/enc_bfv_e_sm/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_sk/src/main.nrcircuits/bin/production/pk_agg_trbfv/src/main.nrcircuits/bin/insecure/greco/src/main.nrcircuits/bin/aggregation/insecure/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/aggregation/pk_trbfv_wrapper/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_e_sm/src/main.nrcircuits/lib/src/core/trbfv_dec_shares_agg.nrcircuits/lib/src/math/helpers.nrcircuits/lib/src/core/trbfv_pk_agg.nrcircuits/bin/insecure/dec_share_trbfv/src/main.nrcircuits/bin/insecure/pk_agg_trbfv/Nargo.tomlcircuits/lib/src/math/commitments.nrcircuits/lib/src/core/trbfv_dec_share.nr
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 133
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:137-139
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs`, using the same RNG instance `rng_test` for generating multiple key shares without advancing its state is acceptable.
Applied to files:
circuits/bin/production/dec_share_agg_trbfv/src/main.nrcircuits/lib/src/core/mod.nrcircuits/bin/insecure/dec_share_agg_trbfv/src/main.nrcircuits/bin/insecure/dec_bfv_e_sm/src/main.nrcircuits/bin/production/dec_share_trbfv/src/main.nrcircuits/bin/insecure/dec_bfv_sk/src/main.nrcircuits/bin/insecure/pk_agg_trbfv/src/main.nrcircuits/bin/production/dec_bfv_sk/src/main.nrcircuits/bin/production/verify_shares_trbfv_sk/src/main.nrcircuits/bin/production/dec_bfv_e_sm/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_sk/src/main.nrcircuits/bin/production/pk_agg_trbfv/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_e_sm/src/main.nrcircuits/lib/src/math/helpers.nrcircuits/bin/production/verify_shares_trbfv_e_sm/src/main.nrcircuits/bin/insecure/dec_share_trbfv/src/main.nr
📚 Learning: 2025-12-17T16:26:24.598Z
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 1122
File: examples/CRISP/crates/crisp-utils/src/lib.rs:30-54
Timestamp: 2025-12-17T16:26:24.598Z
Learning: In the CRISP project's `decode_tally` function (examples/CRISP/crates/crisp-utils/src/lib.rs), the decoded u64 values from `decode_bytes_to_vec_u64` are not necessarily binary (0 or 1). They can be any u64 value and are used as coefficients in a weighted sum with powers of 2.
Applied to files:
circuits/bin/production/dec_share_agg_trbfv/src/main.nrcircuits/bin/insecure/dec_share_agg_trbfv/src/main.nr
📚 Learning: 2025-12-23T17:18:32.343Z
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 1142
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:141-147
Timestamp: 2025-12-23T17:18:32.343Z
Learning: When verifying Noir circuit proofs in Solidity, the public inputs array passed to the verifier must include both the circuit's public inputs AND public outputs. For example, if a Noir circuit has 3 public inputs and returns 2064 public output fields (-> pub [Field; 2064]), the verification array must contain all 2067 values (3 + 2064).
Applied to files:
circuits/bin/production/verify_shares_trbfv_sk/README.mdcircuits/bin/aggregation/production/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/production/pk_agg_trbfv/src/main.nrcircuits/lib/src/core/bfv_pk.nrcircuits/bin/production/pk_bfv/src/main.nrcircuits/bin/aggregation/insecure/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/aggregation/pk_trbfv_wrapper/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_e_sm/src/main.nrcircuits/lib/src/core/greco.nrcircuits/bin/production/verify_shares_trbfv_e_sm/src/main.nrcircuits/lib/src/core/trbfv_pk.nr
📚 Learning: 2024-10-23T01:59:42.967Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:274-274
Timestamp: 2024-10-23T01:59:42.967Z
Learning: In the `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs` file and other test files within this project, hardcoding `CIPHERNODE_SECRET` is acceptable for testing purposes.
Applied to files:
circuits/bin/production/dec_share_trbfv/src/main.nrcircuits/lib/src/configs/mod.nrcircuits/lib/src/math/helpers.nrcircuits/bin/insecure/dec_share_trbfv/src/main.nr
📚 Learning: 2025-11-07T16:17:58.988Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 972
File: circuits/crates/libs/greco/src/lib.nr:192-192
Timestamp: 2025-11-07T16:17:58.988Z
Learning: In the Greco library (circuits/crates/libs/greco/src/lib.nr), the e1is array should satisfy the constraint `e1 mod qi == e1is[i]` for each CRT basis i, where qi are the CRT moduli. This constraint ensures the relationship between the global error polynomial e1 and its per-basis representations e1is[i].
Applied to files:
circuits/bin/production/greco/src/main.nrcircuits/bin/insecure/greco/src/main.nrcircuits/lib/src/core/greco.nrexamples/CRISP/circuits/src/main.nr
📚 Learning: 2025-08-27T13:49:48.617Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: examples/CRISP/circuits/src/main.nr:40-44
Timestamp: 2025-08-27T13:49:48.617Z
Learning: In CRISP circuits using Greco library, the binary check `assert(0 == b * (qmt - b))` for polynomial k1 should not be applied to all coefficients - only specific coefficients require this constraint, such as `k1.coefficients[2048 - 1]`.
Applied to files:
circuits/bin/insecure/greco/README.mdcircuits/lib/src/core/greco.nrexamples/CRISP/circuits/src/main.nr
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: tests/basic_integration/test.sh:103-114
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `tests/basic_integration/test.sh`, the user prefers not to refactor the ciphernode addition section to reduce duplication.
Applied to files:
scripts/test-circuits.sh
📚 Learning: 2024-11-25T09:47:48.863Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 184
File: packages/ciphernode/net/tests/entrypoint.sh:4-8
Timestamp: 2024-11-25T09:47:48.863Z
Learning: When reviewing test scripts like `packages/ciphernode/net/tests/entrypoint.sh`, avoid suggesting additional error handling and cleanup for `iptables` commands, as it may not be necessary.
Applied to files:
scripts/test-circuits.sh
📚 Learning: 2024-11-25T09:48:29.068Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 184
File: packages/ciphernode/net/tests/run.sh:5-8
Timestamp: 2024-11-25T09:48:29.068Z
Learning: In the `run.sh` script in `packages/ciphernode/net/tests`, adding programmatic validation of test results is not appropriate.
Applied to files:
scripts/test-circuits.sh
📚 Learning: 2025-12-23T17:18:22.421Z
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 1142
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:141-147
Timestamp: 2025-12-23T17:18:22.421Z
Learning: For Solidity contracts that verify Noir circuit proofs, the verifier's public inputs array must include both the circuit's public inputs and the public outputs. Ensure the verification input array length equals pub_inputs + pub_outputs. Example: a Noir circuit with 3 public inputs and 2064 public outputs should pass an array of length 2067 (3 + 2064). Apply this consistently across all contract verifier implementations and adjust tests accordingly.
Applied to files:
examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol
📚 Learning: 2024-10-12T10:24:07.572Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 139
File: packages/ciphernode/aggregator/src/publickey_aggregator.rs:46-46
Timestamp: 2024-10-12T10:24:07.572Z
Learning: In `packages/ciphernode/router/src/hooks.rs`, the `src_chain_id` parameter in `PublicKeyAggregator::new` is correctly handled, even if not explicitly provided during instantiation.
Applied to files:
circuits/lib/src/core/trbfv_pk_agg.nr
📚 Learning: 2024-10-29T01:04:19.137Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/keyshare/src/keyshare.rs:53-70
Timestamp: 2024-10-29T01:04:19.137Z
Learning: In the `Keyshare` struct within `packages/ciphernode/keyshare/src/keyshare.rs`, the `self.secret` field is stored in encrypted form and is not considered sensitive.
Applied to files:
circuits/bin/insecure/dec_share_trbfv/src/main.nr
📚 Learning: 2024-10-23T02:35:07.689Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/keyshare/src/keyshare.rs:58-65
Timestamp: 2024-10-23T02:35:07.689Z
Learning: In `packages/ciphernode/keyshare/src/keyshare.rs`, the data being decrypted in the `get_secret` method of the `Keyshare` struct is not sensitive, so wrapping it with `Zeroizing` is unnecessary.
Applied to files:
circuits/bin/insecure/dec_share_trbfv/src/main.nr
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build_enclave_cli
- GitHub Check: build_crisp_sdk
- GitHub Check: build_sdk
- GitHub Check: test_net
- GitHub Check: integration_prebuild
- GitHub Check: rust_integration
- GitHub Check: rust_unit
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
596-602: Fix artifact upload path: artifacts likely created in circuits/lib/target/, not circuits/target/.The
test-circuits.shscript changes tocircuits/libbefore runningnargo test --workspace, which means nargo creates artifacts incircuits/lib/target/, notcircuits/target/. The upload path should be updated tocircuits/lib/target/or the test script should be modified to preserve artifacts in the expected location.
🤖 Fix all issues with AI agents
In `@circuits/bin/aggregation/pk_trbfv_wrapper/Nargo.toml`:
- Around line 1-9: The dependency bb_proof_verification in the pk_trbfv_wrapper
package is pinned to a nightly tag (bb_proof_verification = { git = "...", tag =
"v3.0.0-nightly.20260102", directory = "barretenberg/noir/bb_proof_verification"
}); once a stable v3.0.0 is released, update this dependency (and the same
entries in the other three modules: fold, both verify_shares_trbfv_wrapper
variants) to use the stable tag "v3.0.0" instead of the nightly tag to avoid
pre-release volatility, ensuring consistency across packages and updating the
tag value in each package's Nargo.toml dependency block.
In `@circuits/bin/insecure/dec_share_agg_trbfv/Nargo.toml`:
- Around line 1-5: The package name in the manifest ("name" =
"dec_shares_agg_trbfv") doesn't match the directory/expected package name;
update the name field in Nargo.toml to the correct identifier
("dec_share_agg_trbfv") so workspace resolution and script lookups align with
the directory and other references (change the value of the name key in the
[package] section).
In `@circuits/bin/insecure/enc_bfv_e_sm/src/main.nr`:
- Around line 14-36: The parameters expected_pk_commitment and
expected_message_commitment in the main function are currently private; make
them public inputs by changing their parameter declarations to be public (e.g.,
pub expected_pk_commitment: Field and pub expected_message_commitment: Field) so
the verifier can check the commitments; update any call sites/tests that
construct the proof to pass those public inputs if needed and ensure
EncryptionBfv::new(...) still receives them in the same order.
In `@circuits/bin/insecure/verify_shares_trbfv_sk/src/main.nr`:
- Around line 18-30: The parameter expected_secret_commitment in the main
function is currently a private Field, so the verifier cannot bind the proof to
the externally-known commitment; change its declaration to be a public input
(make expected_secret_commitment pub) so the circuit receives it as a public
input and the rest of the logic (e.g., construction of VerifySharesSk via
VerifySharesSk::new) will be checked against that public commitment.
In `@circuits/bin/production/dec_share_agg_trbfv/Nargo.toml`:
- Around line 1-9: The package name in Nargo.toml is inconsistent with the
directory name: change the [package] name value from "dec_shares_agg_trbfv" to
"dec_share_agg_trbfv" in the Nargo.toml located for both the production and
insecure variants so the package name (name = "dec_share_agg_trbfv") matches the
directory; ensure you update the same key in
circuits/bin/production/dec_share_agg_trbfv/Nargo.toml and
circuits/bin/insecure/dec_share_agg_trbfv/Nargo.toml (referenced symbol: package
name "dec_shares_agg_trbfv").
In `@circuits/bin/production/pk_agg_trbfv/src/main.nr`:
- Around line 14-23: The parameter expected_pk_trbfv_commitments should be a
public input so the proof is bound to externally known commitments; update the
main function signature to declare expected_pk_trbfv_commitments as a public
input (e.g., change the parameter to pub expected_pk_trbfv_commitments: [Field;
H]) and ensure the same public value is passed into
TrbfvPublicKeyAggregation::new and any downstream uses (symbols: main,
expected_pk_trbfv_commitments, TrbfvPublicKeyAggregation).
In `@circuits/lib/src/configs/insecure/trbfv.nr`:
- Around line 117-120: The GRECO bounds are inverted: GRECO_K1_LOW_BOUND (5) is
greater than GRECO_K1_UP_BOUND (4) and GRECO_R1_LOW_BOUNDS[0] (261) is greater
than GRECO_R1_UP_BOUNDS[0] (260); update the constants so the low bounds are <=
the up bounds by swapping the values — set GRECO_K1_LOW_BOUND to 4 and
GRECO_K1_UP_BOUND to 5, and swap the first elements so GRECO_R1_LOW_BOUNDS
becomes [260, 258] and GRECO_R1_UP_BOUNDS becomes [261, 258].
In `@circuits/lib/src/configs/production/trbfv.nr`:
- Around line 123-126: The GRECO bound definitions are inverted:
GRECO_K1_LOW_BOUND (currently 50) is greater than GRECO_K1_UP_BOUND (49) and
each GRECO_R1_LOW_BOUNDS element is greater than the corresponding
GRECO_R1_UP_BOUNDS element; swap the low and up values so lows are <= highs.
Edit the declarations for GRECO_K1_LOW_BOUND and GRECO_K1_UP_BOUND and for
GRECO_R1_LOW_BOUNDS / GRECO_R1_UP_BOUNDS to reorder the numeric values (or
rename if intended) so that GRECO_K1_LOW_BOUND <= GRECO_K1_UP_BOUND and for all
i GRECO_R1_LOW_BOUNDS[i] <= GRECO_R1_UP_BOUNDS[i], keeping the array lengths and
types unchanged.
- Around line 114-115: The Configs struct includes q_mod_t_mod_p but Greco
verification never uses it and GRECO_Q_MOD_T is inconsistently defined between
insecure (3) and production (Q_MOD_T_MOD_P) causing a naming mismatch; either
remove q_mod_t_mod_p and GRECO_Q_MOD_T if unused, or explicitly document and
align names/values: update greco.nr to reference and explain why GRECO_Q_MOD_T
must equal Q_MOD_T_MOD_P in production (or change production to Q_MOD_T), and
make the Configs field name and the global constant (GRECO_Q_MOD_T) consistent
with Q_MOD_T/Q_MOD_T_MOD_P so the intent is clear (check symbols: Configs,
q_mod_t_mod_p, GRECO_Q_MOD_T, Q_MOD_T, Q_MOD_T_MOD_P).
In `@circuits/lib/src/core/bfv_enc.nr`:
- Around line 288-324: check_range_bounds currently omits range checks for the
CRT decomposition pieces e0is and e0_quotients, letting provers shift by
multiples of q_i; add explicit range checks for those fields inside
check_range_bounds (alongside the existing loop over i) so the decomposition is
canonical. Concretely, for each i call the appropriate range_check_2bounds on
self.e0is[i] and on self.e0_quotients[i] (similar to how
self.r1is/r2is/p1is/p2is are checked), using either new config entries (e.g.
configs.e0is_bounds[i] and configs.e0_quotient_bounds[i]) or deterministic
bounds derived from configs.e0_bound and the modulus q_i; ensure you pick/define
BIT types for the bit-widths (e.g. BIT_E0I / BIT_EQ) consistent with existing
BIT_* usage and add those config fields if missing.
In `@circuits/lib/src/core/trbfv_pk_agg.nr`:
- Around line 33-35: Update the documentation comment for the struct/field
expected_pk_trbfv_commitments: change the word "comment" to "commitment" so the
line reads that we need one commitment from each honest party (H) for the
expected_pk_trbfv_commitments (TRBFV from C1); ensure only the typo in the
comment is fixed and no code behavior is altered.
- Around line 114-118: Update the comment on the aggregation verification loop
to refer to pk0 instead of pk (i.e., change "Verify pk & pk1" to "Verify pk0 &
pk1") so it accurately reflects the code calling verify_pk_for_basis(self.pk0,
self.pk0_agg, basis_idx) and verify_pk_for_basis(self.pk1, self.pk1_agg,
basis_idx); no logic changes required, only correct the comment text near the
loop that iterates basis_idx and calls verify_pk_for_basis.
In `@circuits/lib/src/core/trbfv_pk.nr`:
- Around line 111-116: The CRS polynomial array self.a is being flattened with
BIT_EEK but must be flattened with BIT_PK to avoid non-injective packing; update
the call flatten::<_, _, BIT_EEK>(inputs, self.a) to use BIT_PK instead (i.e.,
flatten::<_, _, BIT_PK>(inputs, self.a)) so that inputs packing for self.a
matches the public-key scale and prevents collisions in the Fiat–Shamir
transcript.
In `@circuits/lib/src/math/modulo/U128.nr`:
- Around line 63-73: In the reduce_mod function, update the inline comment that
currently reads "Ensure remainder is in [0, q)" to correctly state the range
"[0, m)" since the assertion checks r < self.m; modify the comment near the
assert(r as u128 < self.m as u128) in reduce_mod to reference m instead of q so
it accurately describes the enforced invariant for r.
In `@circuits/lib/src/math/modulo/unconstrained_U128.nr`:
- Around line 110-115: The __mul_with_quotient function can silently wrap
because Field multiplication is modulo the field prime; either document the safe
input range for lhs/rhs (e.g., must be < 2^128 and such that lhs*rhs <
FIELD_MODULUS) or, preferably, add a debug-mode precondition in
__mul_with_quotient that checks lhs and rhs are small enough to guarantee lhs *
rhs < FIELD_MODULUS (or that their u128 interpretations are below a safe bound)
and panic/log if violated; reference the __mul_with_quotient implementation and
the __compute_mod_reduction call so the check runs before invoking
__compute_mod_reduction.
🧹 Nitpick comments (22)
scripts/test-circuits.sh (1)
4-4: Make the script resilient to execution from any working directory.Using a relative
cdassumes the script is always invoked from the repo root. Consider anchoring the path to the script location so local runs from other directories don’t fail.♻️ Proposed refactor
-cd circuits/lib +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cd "${SCRIPT_DIR}/../circuits/lib"circuits/bin/aggregation/insecure/verify_shares_trbfv_wrapper/Nargo.toml (1)
7-9: Consider pinning to a specific commit for reproducibility.The
bb_proof_verificationdependency uses a nightly tag which may change over time. For improved reproducibility, consider pinning to a specific commit using therevparameter instead of relying on the tag alone.examples/CRISP/circuits/Nargo.toml (1)
9-11: Potential version mismatch forkeccak256dependency.The CRISP circuit uses
keccak256atv0.1.0, while thelibdependency (which is also imported here) usesv0.1.1. This inconsistency could cause issues if Noir doesn't properly deduplicate transitive dependencies, or lead to unexpected behavior if the versions are incompatible.Consider aligning the version to
v0.1.1to match the library:♻️ Suggested fix
-keccak256 = { tag = "v0.1.0", git = "https://github.com/noir-lang/keccak256" } +keccak256 = { tag = "v0.1.1", git = "https://github.com/noir-lang/keccak256" }scripts/lint-circuits.sh (1)
26-33: Unusedcreated_filesarray.The
created_filesarray is populated but never used. Line 48 removes allProver.tomlfiles viafind, not just the ones tracked in this array.Either remove the unused tracking or use it for cleanup:
Option 1: Remove unused tracking
# Find all package directories and create empty Prover.toml to prevent nargo from creating them - created_files=() while IFS= read -r -d '' pkg_dir; do prover_file="${pkg_dir}/Prover.toml" if [ ! -f "$prover_file" ]; then touch "$prover_file" - created_files+=("$prover_file") fi done < <(find . -name "Nargo.toml" -type f -print0 2>/dev/null | xargs -0 dirname 2>/dev/null || true)Option 2: Use tracked files for cleanup (safer)
- # Remove all Prover.toml files (they're generated by nargo and in .gitignore) - find . -name "Prover.toml" -type f -delete + # Remove only the Prover.toml files we created + for f in "${created_files[@]}"; do + rm -f "$f" + donecircuits/bin/insecure/dec_share_trbfv/README.md (1)
1-1: Consider expanding documentation.The README is minimal. For clarity, consider:
- Explaining what "insecure" means in this context (e.g., smaller parameters for testing)
- Linking or explaining the "PVSS
#6" reference- Adding usage examples or cross-references to production counterpart
circuits/lib/src/math/modulo/unconstrained_U128.nr (2)
53-59: Potential edge case:val >= mnot handled.If
val >= m, the computationm - valwould produce an incorrect result (Field underflow wrapping to a large value). The constrainedneg()inU128.nrwould catch this via its assertion, but consider documenting the precondition thatvalshould be in[0, m).📝 Suggested documentation addition
/// # Arguments /// * `val` - The value to negate +/// * `val` must be in the range [0, m) for correct results /// * `m` - The modulus
131-134: Fermat's Little Theorem requires prime modulus.The implementation uses
val^(m-2) mod m, which only produces the correct inverse whenmis prime. For composite moduli, this silently returns an incorrect value. The constrainedinv_modinU128.nrwill catch this via verification, but consider making the prime requirement more prominent in the documentation.📝 Suggested documentation enhancement
/// # Arguments /// * `val` - The value to invert (must be coprime with the modulus) -/// * `m` - The modulus (should be prime for this method to work correctly) +/// * `m` - The modulus (**must be prime** for this method to work correctly)circuits/lib/src/math/modulo/U128.nr (2)
38-40: Consider validating modulus is non-zero.The constructor accepts any
Fieldvalue including zero. A zero modulus would cause runtime failures in operations likereduce_mod. Consider adding an assertion:🛡️ Suggested defensive check
pub fn new(m: Field) -> Self { + assert(m != 0, "Modulus must be non-zero"); ModU128 { m } }
168-181: Precondition:valmust be in[0, m)for correct negation.The assertion
val + result == self.monly holds whenval < m. Ifval >= m, the unconstrained__negreturnsm - val(which wraps for Field), and the assertion would fail unexpectedly. Consider either:
- Documenting the precondition explicitly
- Pre-reducing
valbefore negation🛡️ Option: Pre-reduce val
pub fn neg(self, val: Field) -> Field { - // Safety: __neg is safe because we verify val + result = 0 (mod m) below - let result = unsafe { __neg(val, self.m) }; + // First reduce val to ensure it's in [0, m) + let reduced_val = self.reduce_mod(val); + // Safety: __neg is safe because we verify reduced_val + result = 0 (mod m) below + let result = unsafe { __neg(reduced_val, self.m) }; // Verify: val + result = 0 (mod m) // Special case: if val = 0, result should be 0 - if val == 0 { + if reduced_val == 0 { assert(result == 0, "Negation of zero should be zero"); } else { - assert(val + result == self.m, "Negation verification failed"); + assert(reduced_val + result == self.m, "Negation verification failed"); } result }circuits/bin/insecure/dec_share_trbfv/src/main.nr (1)
14-17: Remove or document the unused global constantsN_PARTIESandT.These public globals are defined but never referenced in this file. Unlike other circuit implementations in the codebase (e.g.,
verify_shares_trbfv_sk), theDecryptionShareverification in this circuit doesn't require these parameters. Either remove them or add a doc comment explaining their purpose if they're intended as documentation.circuits/bin/insecure/verify_shares_trbfv_sk/README.md (1)
1-1: Expand README with comprehensive circuit documentation.This single-line README provides minimal value for a security-critical cryptographic circuit. Consider including:
- Purpose: Detailed explanation of what this circuit verifies and its role in PVSS
- Security notice: Clear explanation of why this is "insecure" and when it should/shouldn't be used
- Interface: Document public inputs, outputs, and their formats
- Usage: Examples of how to build/run the circuit
- PVSS context: Explain what "PVSS
#2a" refers to and link to relevant specifications- Differences: How this differs from the production variant
This applies to cryptographic circuits where insufficient documentation can lead to misuse.
circuits/bin/production/greco/README.md (1)
1-1: Expand README with comprehensive circuit documentation.This single-line README is insufficient for a production cryptographic circuit. Include:
- Purpose: What the Greco circuit does and its cryptographic properties
- Production vs Insecure: Explain what makes this production-ready (e.g., parameter sizes, security levels)
- Configuration: Document the GRECO_CONFIGS and GRECO_BIT_* constants mentioned in the implementation
- Interface: Public inputs/outputs specification
- Usage: Build and integration instructions
- References: Link to Greco protocol specifications or papers
Comprehensive documentation is essential for production cryptographic components to prevent misuse and facilitate auditing.
circuits/bin/production/dec_share_agg_trbfv/README.md (1)
1-1: Expand README with comprehensive circuit documentation.This minimal README is insufficient for a production threshold cryptography circuit. Document:
- Purpose: Explain decryption share aggregation in the TRBFV/PVSS protocol
- Parameters: Document MAX_MSG_NON_ZERO_COEFFS (80) and threshold T (2) and their security implications
- Interface: Specify inputs (decryption_shares, party_ids, message, u_global, crt_quotients) and outputs
- Security: Production security guarantees and parameter choices
- Threshold requirements: Explain the T=2 threshold and how shares combine
- PVSS
#7context: Link to specification or explain the protocol step- Usage: Integration examples and verification requirements
Threshold cryptography components require detailed documentation to ensure correct and secure usage.
circuits/bin/production/pk_agg_trbfv/README.md (1)
1-1: Expand README with comprehensive circuit documentation.While this README uses proper title case (better than others), it still lacks essential documentation for a production threshold cryptography circuit. Include:
- Purpose: Explain public key aggregation in TRBFV and why it's needed
- Public parameters: Document the public global H and its cryptographic role
- Interface: Input/output specifications (public keys, aggregation result)
- Security: Production-grade parameter choices and security properties
- PVSS
#5context: Explain this protocol step in the overall PVSS workflow- Verification: How aggregated keys are verified and used
- Usage: Integration guide with examples
Production cryptographic primitives need comprehensive documentation for secure deployment and auditing.
circuits/bin/insecure/dec_share_agg_trbfv/README.md (1)
1-1: Expand README with comprehensive circuit documentation, especially insecure vs production differences.This minimal README is particularly problematic for an "insecure" variant. Document:
- Purpose: Decryption share aggregation in TRBFV/PVSS
⚠️ Insecurity warning: Clearly explain why this is "insecure" and when it should NOT be used- Key difference: This uses
verify_no_bn()while production usesverify()- explain the security trade-off- Parameters: Document MAX_MSG_NON_ZERO_COEFFS (80) and T (2)
- Interface: Inputs (decryption_shares, party_ids, message, u_global, crt_quotients) and outputs
- Testing/development use: When this variant is appropriate (e.g., faster testing)
- Migration path: How to transition to production variant
- PVSS
#7context: Protocol step explanationBased on learnings, while individual decryption shares aren't sensitive, the circuit's security properties still require clear documentation to prevent misuse in production.
circuits/bin/insecure/pk_bfv/README.md (1)
1-1: LGTM - Consider adding usage context (optional).The README clearly identifies this as an insecure instantiation. For improved developer onboarding, consider optionally adding a brief note about the purpose of insecure instantiations (e.g., "for testing/development only") and when to use production variants instead.
circuits/bin/insecure/greco/README.md (1)
1-1: Consider adding PVSS identifier for consistency.Unlike other insecure instantiation READMEs in this PR (which include PVSS
#0,#1,#3b), this README doesn't specify a PVSS number. If Greco is part of the PVSS circuit suite, consider adding the identifier for consistency. If it's intentionally separate from the PVSS numbering scheme, the current documentation is acceptable.circuits/bin/production/Nargo.toml (1)
1-15: LGTM!The workspace manifest correctly organizes all production circuit crates covering the BFV/TRBFV workflow (public key, encryption, decryption, verification, aggregation, and Greco circuits).
Minor nit: Line 14 has trailing whitespace after
"dec_share_agg_trbfv".🧹 Remove trailing whitespace
- "dec_share_agg_trbfv" + "dec_share_agg_trbfv"circuits/bin/aggregation/production/verify_shares_trbfv_wrapper/src/main.nr (1)
10-13: Minor: Inconsistent comment style.Line 10 uses
//while line 12 uses///(doc comment). Consider using consistent doc comment style for both globals.Suggested fix
-// Number of proofs. +/// Number of proofs. pub global N_PROOFS: u32 = 2; /// Number of commitments per proof. pub global N_COMMITMENTS: u32 = 20;circuits/lib/src/math/polynomial.nr (1)
123-142: Remove test-only function or add guard for u128 moduli.The comparison
acc as u64 >= m as u64(lines 131, 137) truncates Field values exceeding 2^64. WhileModU128is designed to support moduli up to 2^128,eval_modonly handles values fitting in u64.This is currently safe:
eval_modexists only in test functions using moduli ≤ ~58 bits. However, if this function is intended for production use (e.g., Greco circuit evaluation), it should either:
- Document the u64 limitation explicitly and enforce it with a compile-time check, or
- Implement comparison using
ModU128's native methods that handle the full u128 range.circuits/lib/src/core/trbfv_pk.nr (1)
194-221: Avoid evaluating all polynomials per modulus.
verify_public_key_for_modulusmaps over alla/r1/r2/pk*each iteration, yielding O(L²) evaluations. Index directly (or precompute once outside the loop) to keep constraints linear in L.♻️ Suggested refactor (index directly)
- let a_at_gamma = self.a.map(|a_poly| a_poly.eval(gamma)); + let a_at_gamma = self.a[i].eval(gamma); ... - let r1_at_gamma = self.r1.map(|r1_poly| r1_poly.eval(gamma)); - let r2_at_gamma = self.r2.map(|r2_poly| r2_poly.eval(gamma)); + let r1_at_gamma = self.r1[i].eval(gamma); + let r2_at_gamma = self.r2[i].eval(gamma); ... - let pk0_at_gamma = self.pk0.map(|pk0_poly| pk0_poly.eval(gamma)); - let pk1_at_gamma = self.pk1.map(|pk1_poly| pk1_poly.eval(gamma)); + let pk0_at_gamma = self.pk0[i].eval(gamma); + let pk1_at_gamma = self.pk1[i].eval(gamma); ... - let expected_pk0 = -a_at_gamma[i] * sk_at_gamma + let expected_pk0 = -a_at_gamma * sk_at_gamma + eek_at_gamma - + r2_at_gamma[i] * cyclo_at_gamma - + r1_at_gamma[i] * self.configs.qis[i]; + + r2_at_gamma * cyclo_at_gamma + + r1_at_gamma * self.configs.qis[i]; ... - assert(pk0_at_gamma[i] == expected_pk0, "Public key equation 1 failed"); - assert(pk1_at_gamma[i] == a_at_gamma[i], "Public key equation 2 failed"); + assert(pk0_at_gamma == expected_pk0, "Public key equation 1 failed"); + assert(pk1_at_gamma == a_at_gamma, "Public key equation 2 failed");circuits/lib/src/math/commitments.nr (1)
93-191: Consider centralizing domain separators as constants.There’s a lot of repeated 64-byte domain separator data. Extracting them into
constvalues (or a small helper) would reduce copy/paste risk and make audits easier.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (125)
.github/workflows/ci.ymlcircuits/Nargo.tomlcircuits/bin/aggregation/Nargo.tomlcircuits/bin/aggregation/fold/Nargo.tomlcircuits/bin/aggregation/fold/src/main.nrcircuits/bin/aggregation/insecure/verify_shares_trbfv_wrapper/Nargo.tomlcircuits/bin/aggregation/insecure/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/aggregation/pk_trbfv_wrapper/Nargo.tomlcircuits/bin/aggregation/pk_trbfv_wrapper/src/main.nrcircuits/bin/aggregation/production/verify_shares_trbfv_wrapper/Nargo.tomlcircuits/bin/aggregation/production/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/insecure/Nargo.tomlcircuits/bin/insecure/dec_bfv_e_sm/Nargo.tomlcircuits/bin/insecure/dec_bfv_e_sm/README.mdcircuits/bin/insecure/dec_bfv_e_sm/src/main.nrcircuits/bin/insecure/dec_bfv_sk/Nargo.tomlcircuits/bin/insecure/dec_bfv_sk/README.mdcircuits/bin/insecure/dec_bfv_sk/src/main.nrcircuits/bin/insecure/dec_share_agg_trbfv/Nargo.tomlcircuits/bin/insecure/dec_share_agg_trbfv/README.mdcircuits/bin/insecure/dec_share_agg_trbfv/src/main.nrcircuits/bin/insecure/dec_share_trbfv/Nargo.tomlcircuits/bin/insecure/dec_share_trbfv/README.mdcircuits/bin/insecure/dec_share_trbfv/src/main.nrcircuits/bin/insecure/enc_bfv_e_sm/Nargo.tomlcircuits/bin/insecure/enc_bfv_e_sm/README.mdcircuits/bin/insecure/enc_bfv_e_sm/src/main.nrcircuits/bin/insecure/enc_bfv_sk/Nargo.tomlcircuits/bin/insecure/enc_bfv_sk/README.mdcircuits/bin/insecure/enc_bfv_sk/src/main.nrcircuits/bin/insecure/greco/Nargo.tomlcircuits/bin/insecure/greco/README.mdcircuits/bin/insecure/greco/src/main.nrcircuits/bin/insecure/pk_agg_trbfv/Nargo.tomlcircuits/bin/insecure/pk_agg_trbfv/README.mdcircuits/bin/insecure/pk_agg_trbfv/src/main.nrcircuits/bin/insecure/pk_bfv/Nargo.tomlcircuits/bin/insecure/pk_bfv/README.mdcircuits/bin/insecure/pk_bfv/src/main.nrcircuits/bin/insecure/pk_trbfv/Nargo.tomlcircuits/bin/insecure/pk_trbfv/README.mdcircuits/bin/insecure/pk_trbfv/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_e_sm/Nargo.tomlcircuits/bin/insecure/verify_shares_trbfv_e_sm/README.mdcircuits/bin/insecure/verify_shares_trbfv_e_sm/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_sk/Nargo.tomlcircuits/bin/insecure/verify_shares_trbfv_sk/README.mdcircuits/bin/insecure/verify_shares_trbfv_sk/src/main.nrcircuits/bin/production/Nargo.tomlcircuits/bin/production/dec_bfv_e_sm/Nargo.tomlcircuits/bin/production/dec_bfv_e_sm/README.mdcircuits/bin/production/dec_bfv_e_sm/src/main.nrcircuits/bin/production/dec_bfv_sk/Nargo.tomlcircuits/bin/production/dec_bfv_sk/README.mdcircuits/bin/production/dec_bfv_sk/src/main.nrcircuits/bin/production/dec_share_agg_trbfv/Nargo.tomlcircuits/bin/production/dec_share_agg_trbfv/README.mdcircuits/bin/production/dec_share_agg_trbfv/src/main.nrcircuits/bin/production/dec_share_trbfv/Nargo.tomlcircuits/bin/production/dec_share_trbfv/README.mdcircuits/bin/production/dec_share_trbfv/src/main.nrcircuits/bin/production/enc_bfv_e_sm/Nargo.tomlcircuits/bin/production/enc_bfv_e_sm/README.mdcircuits/bin/production/enc_bfv_e_sm/src/main.nrcircuits/bin/production/enc_bfv_sk/Nargo.tomlcircuits/bin/production/enc_bfv_sk/README.mdcircuits/bin/production/enc_bfv_sk/src/main.nrcircuits/bin/production/greco/Nargo.tomlcircuits/bin/production/greco/README.mdcircuits/bin/production/greco/src/main.nrcircuits/bin/production/pk_agg_trbfv/Nargo.tomlcircuits/bin/production/pk_agg_trbfv/README.mdcircuits/bin/production/pk_agg_trbfv/src/main.nrcircuits/bin/production/pk_bfv/Nargo.tomlcircuits/bin/production/pk_bfv/README.mdcircuits/bin/production/pk_bfv/src/main.nrcircuits/bin/production/pk_trbfv/Nargo.tomlcircuits/bin/production/pk_trbfv/README.mdcircuits/bin/production/pk_trbfv/src/main.nrcircuits/bin/production/verify_shares_trbfv_e_sm/Nargo.tomlcircuits/bin/production/verify_shares_trbfv_e_sm/README.mdcircuits/bin/production/verify_shares_trbfv_e_sm/src/main.nrcircuits/bin/production/verify_shares_trbfv_sk/Nargo.tomlcircuits/bin/production/verify_shares_trbfv_sk/README.mdcircuits/bin/production/verify_shares_trbfv_sk/src/main.nrcircuits/crates/libs/greco/Nargo.tomlcircuits/crates/libs/greco/README.mdcircuits/crates/libs/polynomial/Nargo.tomlcircuits/crates/libs/polynomial/README.mdcircuits/crates/libs/safe/README.mdcircuits/lib/Nargo.tomlcircuits/lib/src/configs/insecure/bfv.nrcircuits/lib/src/configs/insecure/mod.nrcircuits/lib/src/configs/insecure/trbfv.nrcircuits/lib/src/configs/mod.nrcircuits/lib/src/configs/production/bfv.nrcircuits/lib/src/configs/production/mod.nrcircuits/lib/src/configs/production/trbfv.nrcircuits/lib/src/core/bfv_dec.nrcircuits/lib/src/core/bfv_enc.nrcircuits/lib/src/core/bfv_pk.nrcircuits/lib/src/core/greco.nrcircuits/lib/src/core/mod.nrcircuits/lib/src/core/trbfv_dec_share.nrcircuits/lib/src/core/trbfv_dec_shares_agg.nrcircuits/lib/src/core/trbfv_pk.nrcircuits/lib/src/core/trbfv_pk_agg.nrcircuits/lib/src/core/trbfv_verify_shares.nrcircuits/lib/src/lib.nrcircuits/lib/src/math/commitments.nrcircuits/lib/src/math/helpers.nrcircuits/lib/src/math/mod.nrcircuits/lib/src/math/modulo/U128.nrcircuits/lib/src/math/modulo/mod.nrcircuits/lib/src/math/modulo/unconstrained_U128.nrcircuits/lib/src/math/polynomial.nrcircuits/lib/src/math/safe.nrexamples/CRISP/circuits/Nargo.tomlexamples/CRISP/circuits/src/ciphertext_addition.nrexamples/CRISP/circuits/src/hash.nrexamples/CRISP/circuits/src/main.nrexamples/CRISP/circuits/src/utils.nrexamples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.solscripts/lint-circuits.shscripts/test-circuits.sh
💤 Files with no reviewable changes (6)
- circuits/crates/libs/safe/README.md
- circuits/crates/libs/polynomial/README.md
- circuits/crates/libs/greco/README.md
- circuits/Nargo.toml
- circuits/crates/libs/polynomial/Nargo.toml
- circuits/crates/libs/greco/Nargo.toml
🧰 Additional context used
🧠 Learnings (18)
📓 Common learnings
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: packages/circuits/crates/libs/polynomial/src/lib.nr:121-133
Timestamp: 2025-08-27T13:53:47.832Z
Learning: In Noir circuits, prefer relying on built-in array bounds checking rather than adding explicit preconditions for buffer overflow protection, as the language already provides safety guarantees that will fail the circuit on out-of-bounds access.
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 1142
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:141-147
Timestamp: 2025-12-23T17:18:32.343Z
Learning: When verifying Noir circuit proofs in Solidity, the public inputs array passed to the verifier must include both the circuit's public inputs AND public outputs. For example, if a Noir circuit has 3 public inputs and returns 2064 public output fields (-> pub [Field; 2064]), the verification array must contain all 2067 values (3 + 2064).
📚 Learning: 2025-10-04T14:00:17.916Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 660
File: crates/events/src/enclave_event/decryptionshare_created.rs:23-26
Timestamp: 2025-10-04T14:00:17.916Z
Learning: In the enclave codebase using threshold BFV cryptography, decryption shares (as in DecryptionshareCreated events) are not sensitive data and can be safely logged or displayed, as individual shares reveal nothing without reaching the threshold number of shares.
Applied to files:
circuits/bin/production/dec_share_trbfv/README.mdcircuits/bin/insecure/verify_shares_trbfv_e_sm/README.mdcircuits/bin/production/verify_shares_trbfv_sk/README.mdcircuits/bin/insecure/dec_bfv_sk/README.mdcircuits/bin/insecure/dec_share_agg_trbfv/src/main.nrcircuits/bin/insecure/dec_share_trbfv/README.mdcircuits/bin/production/dec_share_trbfv/src/main.nrcircuits/bin/insecure/dec_share_agg_trbfv/README.mdcircuits/bin/insecure/verify_shares_trbfv_sk/README.mdcircuits/bin/insecure/dec_share_trbfv/src/main.nrcircuits/lib/src/core/trbfv_dec_shares_agg.nrcircuits/bin/production/dec_share_agg_trbfv/src/main.nrcircuits/lib/src/core/trbfv_dec_share.nr
📚 Learning: 2025-11-08T01:31:47.505Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 969
File: packages/enclave-sdk/src/enclave-sdk.ts:101-103
Timestamp: 2025-11-08T01:31:47.505Z
Learning: In packages/enclave-sdk/src/enclave-sdk.ts, the TRBFV protocol parameter setup is intentionally incomplete as of PR `#969`. The constructor sets BFV_THRESHOLD parameters for TRBFV, but the encryption methods (encryptNumber, encryptVector, encryptNumberAndGenInputs, encryptVectorAndGenInputs) intentionally only support BFV and will throw "Protocol not supported" for TRBFV until future implementation is completed.
Applied to files:
circuits/bin/insecure/enc_bfv_e_sm/README.mdcircuits/bin/insecure/pk_trbfv/README.mdcircuits/bin/insecure/dec_bfv_sk/README.mdcircuits/bin/insecure/pk_bfv/README.mdcircuits/bin/production/enc_bfv_sk/Nargo.tomlcircuits/bin/insecure/dec_share_trbfv/README.mdcircuits/bin/insecure/enc_bfv_sk/README.mdcircuits/bin/insecure/pk_agg_trbfv/README.mdcircuits/bin/insecure/enc_bfv_sk/Nargo.tomlcircuits/lib/src/core/bfv_enc.nrcircuits/lib/src/configs/production/bfv.nrcircuits/lib/src/configs/insecure/bfv.nrcircuits/lib/src/configs/production/trbfv.nrcircuits/lib/src/configs/insecure/trbfv.nr
📚 Learning: 2025-05-07T15:18:20.056Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 388
File: packages/commons/src/bfv/mod.rs:38-48
Timestamp: 2025-05-07T15:18:20.056Z
Learning: The `build_bfv_params` and related functions in the commons BFV utilities intentionally use panic for error handling as a temporary solution. The team plans to refactor these to use proper Result-based error handling in the future.
Applied to files:
circuits/lib/src/configs/production/mod.nrcircuits/lib/src/configs/insecure/mod.nrcircuits/lib/src/core/mod.nrcircuits/bin/insecure/enc_bfv_e_sm/src/main.nr
📚 Learning: 2025-12-23T17:18:32.343Z
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 1142
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:141-147
Timestamp: 2025-12-23T17:18:32.343Z
Learning: When verifying Noir circuit proofs in Solidity, the public inputs array passed to the verifier must include both the circuit's public inputs AND public outputs. For example, if a Noir circuit has 3 public inputs and returns 2064 public output fields (-> pub [Field; 2064]), the verification array must contain all 2067 values (3 + 2064).
Applied to files:
circuits/bin/production/verify_shares_trbfv_sk/README.mdcircuits/bin/aggregation/insecure/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_sk/src/main.nrcircuits/bin/production/verify_shares_trbfv_sk/src/main.nrcircuits/bin/production/enc_bfv_e_sm/src/main.nrcircuits/bin/aggregation/production/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/insecure/enc_bfv_sk/src/main.nrcircuits/bin/aggregation/fold/src/main.nrcircuits/lib/src/core/trbfv_pk_agg.nrcircuits/lib/src/core/greco.nrcircuits/lib/src/core/trbfv_pk.nr
📚 Learning: 2024-10-22T02:36:01.448Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/data/Cargo.toml:0-0
Timestamp: 2024-10-22T02:36:01.448Z
Learning: In `packages/ciphernode/data/Cargo.toml`, `actix-rt` is only used in tests and should remain in `[dev-dependencies]`.
Applied to files:
circuits/bin/insecure/dec_bfv_sk/Nargo.tomlcircuits/bin/production/enc_bfv_sk/Nargo.tomlexamples/CRISP/circuits/Nargo.tomlcircuits/bin/insecure/pk_agg_trbfv/Nargo.toml
📚 Learning: 2025-08-25T10:28:56.174Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 657
File: Cargo.toml:32-34
Timestamp: 2025-08-25T10:28:56.174Z
Learning: The examples/CRISP directory has its own Cargo.toml workspace configuration with members like "server", "wasm-crypto", "program/core", "program/client", etc. The root workspace intentionally excludes "examples/CRISP/server", "examples/CRISP/program", and "examples/CRISP/wasm-crypto" to prevent double workspace membership, which is the correct approach for self-contained example workspaces.
Applied to files:
circuits/bin/aggregation/Nargo.tomlcircuits/bin/insecure/Nargo.tomlcircuits/bin/production/Nargo.tomlexamples/CRISP/circuits/Nargo.toml
📚 Learning: 2025-08-27T13:49:03.811Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: .github/workflows/ci.yml:202-206
Timestamp: 2025-08-27T13:49:03.811Z
Learning: In GitHub Actions workflows for Noir circuits, the user prefers to use "stable" toolchain with noir-lang/noirup action rather than pinning to specific versions like 1.0.0-beta.11, and prefers to update the noirup action version (e.g., to 0.1.4) to get tooling improvements.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 133
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:137-139
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs`, using the same RNG instance `rng_test` for generating multiple key shares without advancing its state is acceptable.
Applied to files:
circuits/bin/production/verify_shares_trbfv_e_sm/src/main.nrcircuits/lib/src/core/mod.nrcircuits/bin/production/pk_agg_trbfv/src/main.nrcircuits/bin/insecure/dec_share_agg_trbfv/src/main.nrcircuits/bin/production/dec_share_trbfv/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_sk/src/main.nrcircuits/bin/production/dec_bfv_sk/src/main.nrcircuits/bin/insecure/dec_bfv_sk/src/main.nrcircuits/bin/insecure/pk_agg_trbfv/src/main.nrcircuits/bin/production/dec_bfv_e_sm/src/main.nrcircuits/bin/insecure/verify_shares_trbfv_e_sm/src/main.nrcircuits/bin/production/verify_shares_trbfv_sk/src/main.nrcircuits/lib/src/math/helpers.nrcircuits/bin/insecure/dec_bfv_e_sm/src/main.nrcircuits/bin/insecure/dec_share_trbfv/src/main.nrcircuits/bin/production/dec_share_agg_trbfv/src/main.nr
📚 Learning: 2024-09-26T03:11:29.311Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-09-26T03:11:29.311Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Applied to files:
examples/CRISP/circuits/src/utils.nrcircuits/lib/src/core/mod.nrexamples/CRISP/circuits/src/hash.nrexamples/CRISP/circuits/src/ciphertext_addition.nrcircuits/lib/src/math/helpers.nrcircuits/lib/Nargo.toml
📚 Learning: 2025-09-22T15:08:29.814Z
Learnt from: ozgurarmanc
Repo: gnosisguild/enclave PR: 734
File: packages/circuits/crates/libs/polynomial/src/lib.nr:140-155
Timestamp: 2025-09-22T15:08:29.814Z
Learning: Greco (packages/circuits/crates/libs/greco/src/lib.nr) performs range_check_1bound/2bounds on all polynomials (u, e0/e1, k1, pk*, r*, p*) before serialization; packer/flatten rely on these bounds, so per-limb asserts inside packer are unnecessary in this crate’s flow.
Applied to files:
examples/CRISP/circuits/src/utils.nrcircuits/bin/production/greco/src/main.nrexamples/CRISP/circuits/src/ciphertext_addition.nrcircuits/lib/src/math/helpers.nrcircuits/lib/src/math/polynomial.nrcircuits/lib/src/math/modulo/unconstrained_U128.nrexamples/CRISP/circuits/Nargo.tomlcircuits/bin/insecure/greco/src/main.nrcircuits/lib/src/core/bfv_enc.nrcircuits/lib/src/core/greco.nrcircuits/lib/src/core/trbfv_pk.nrexamples/CRISP/circuits/src/main.nr
📚 Learning: 2024-10-22T02:10:34.864Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:82-83
Timestamp: 2024-10-22T02:10:34.864Z
Learning: In the file `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs`, when reviewing test functions like `generate_pk_share`, minor performance optimizations (e.g., minimizing mutex locks) are not a priority.
Applied to files:
circuits/lib/src/core/mod.nrcircuits/bin/production/pk_agg_trbfv/src/main.nrcircuits/bin/insecure/dec_share_agg_trbfv/src/main.nrcircuits/bin/production/greco/src/main.nrcircuits/bin/aggregation/insecure/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/production/dec_share_trbfv/src/main.nrcircuits/bin/production/dec_bfv_sk/src/main.nrcircuits/bin/insecure/dec_bfv_sk/src/main.nrcircuits/bin/insecure/pk_agg_trbfv/src/main.nrcircuits/bin/production/dec_bfv_e_sm/src/main.nrexamples/CRISP/circuits/src/ciphertext_addition.nrcircuits/bin/production/enc_bfv_e_sm/src/main.nrcircuits/lib/src/math/helpers.nrcircuits/bin/aggregation/production/verify_shares_trbfv_wrapper/src/main.nrcircuits/bin/aggregation/pk_trbfv_wrapper/src/main.nrcircuits/bin/insecure/enc_bfv_sk/src/main.nrcircuits/bin/production/enc_bfv_sk/src/main.nrcircuits/bin/insecure/dec_bfv_e_sm/src/main.nrcircuits/bin/insecure/greco/src/main.nrcircuits/bin/insecure/enc_bfv_e_sm/src/main.nrcircuits/bin/insecure/dec_share_trbfv/src/main.nrcircuits/lib/src/core/trbfv_dec_shares_agg.nrcircuits/bin/production/dec_share_agg_trbfv/src/main.nrcircuits/lib/src/core/trbfv_dec_share.nr
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: tests/basic_integration/test.sh:103-114
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `tests/basic_integration/test.sh`, the user prefers not to refactor the ciphernode addition section to reduce duplication.
Applied to files:
scripts/test-circuits.sh
📚 Learning: 2024-10-23T01:59:42.967Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:274-274
Timestamp: 2024-10-23T01:59:42.967Z
Learning: In the `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs` file and other test files within this project, hardcoding `CIPHERNODE_SECRET` is acceptable for testing purposes.
Applied to files:
circuits/lib/src/configs/mod.nrcircuits/bin/production/dec_share_trbfv/src/main.nrcircuits/lib/src/math/helpers.nrcircuits/bin/insecure/dec_share_trbfv/src/main.nr
📚 Learning: 2025-12-17T16:26:24.598Z
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 1122
File: examples/CRISP/crates/crisp-utils/src/lib.rs:30-54
Timestamp: 2025-12-17T16:26:24.598Z
Learning: In the CRISP project's `decode_tally` function (examples/CRISP/crates/crisp-utils/src/lib.rs), the decoded u64 values from `decode_bytes_to_vec_u64` are not necessarily binary (0 or 1). They can be any u64 value and are used as coefficients in a weighted sum with powers of 2.
Applied to files:
circuits/bin/insecure/dec_share_agg_trbfv/src/main.nrcircuits/bin/production/dec_share_agg_trbfv/src/main.nr
📚 Learning: 2025-08-27T13:49:48.617Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: examples/CRISP/circuits/src/main.nr:40-44
Timestamp: 2025-08-27T13:49:48.617Z
Learning: In CRISP circuits using Greco library, the binary check `assert(0 == b * (qmt - b))` for polynomial k1 should not be applied to all coefficients - only specific coefficients require this constraint, such as `k1.coefficients[2048 - 1]`.
Applied to files:
circuits/bin/insecure/greco/README.mdcircuits/lib/src/core/greco.nr
📚 Learning: 2025-11-07T16:17:58.988Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 972
File: circuits/crates/libs/greco/src/lib.nr:192-192
Timestamp: 2025-11-07T16:17:58.988Z
Learning: In the Greco library (circuits/crates/libs/greco/src/lib.nr), the e1is array should satisfy the constraint `e1 mod qi == e1is[i]` for each CRT basis i, where qi are the CRT moduli. This constraint ensures the relationship between the global error polynomial e1 and its per-basis representations e1is[i].
Applied to files:
circuits/bin/production/greco/src/main.nrcircuits/bin/insecure/greco/src/main.nrcircuits/lib/src/core/greco.nr
📚 Learning: 2025-12-23T17:18:22.421Z
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 1142
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:141-147
Timestamp: 2025-12-23T17:18:22.421Z
Learning: For Solidity contracts that verify Noir circuit proofs, the verifier's public inputs array must include both the circuit's public inputs and the public outputs. Ensure the verification input array length equals pub_inputs + pub_outputs. Example: a Noir circuit with 3 public inputs and 2064 public outputs should pass an array of length 2067 (3 + 2064). Apply this consistently across all contract verifier implementations and adjust tests accordingly.
Applied to files:
examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: template_integration
- GitHub Check: crisp_e2e
- GitHub Check: ciphernode_integration_test (persist)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Contributor
|
awesome @cedoor - I will get a look at coderabbit suggestions as well! |
cedoor
force-pushed
the
refactor/pvss-migration
branch
from
60b2a2a to
a39ae48
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@circuits/bin/insecure/dec_share_trbfv/src/main.nr`:
- Around line 14-17: The public globals N_PARTIES and T are unused in this
circuit file; either remove their declarations or document why they are exported
(e.g., used by external test harnesses) — if removing, delete the pub global
N_PARTIES and pub global T lines; if keeping, add a one-line clarifying comment
above each explaining their external purpose and link to the DecryptionShare and
verify_shares_trbfv usage so readers know they are intentionally unused inside
this file.
In `@circuits/lib/src/core/trbfv_dec_share.nr`:
- Around line 111-128: The docblock describing the flattening order is
incorrect: it states that aggregated secret shares `s` and noise shares `e` are
serialized, but the implementation only binds them via commitments (they are not
flattened into the array). Update the comment in the flattening function (the
"Flattens all witness data..." doc in trbfv_dec_share.nr) to state that `s` and
`e` are represented by their commitments rather than serialized coefficients (or
alternatively, if you intend to include them, change the implementation to
actually append `s` and `e` coefficients); ensure the list of serialized items
(c_0, c_1, commitments for s/e, r_1, r_2, d) and the description of usage by the
SAFE sponge API match the real behavior.
♻️ Duplicate comments (1)
circuits/lib/src/core/trbfv_dec_shares_agg.nr (1)
229-240: Compute Q directly in BigNum to avoid Field overflow.
q_modulusis accumulated inFieldand then converted to BigNum. For large parameter sets, this can wrap mod the field and silently corrupt decoding verification. Buildq_bndirectly from the CRT moduli.✅ Proposed fix
- // Compute Q as product of all CRT moduli - let mut q_modulus = 1 as Field; - for l in 0..L { - q_modulus *= self.configs.qis[l]; - } - - // For centered arithmetic - let q_bn = Enclave_TRBFV_8192::from(q_modulus); + // Compute Q as product of all CRT moduli in BigNum + let mut q_bn = Enclave_TRBFV_8192::from(1); + for l in 0..L { + q_bn = q_bn * Enclave_TRBFV_8192::from(self.configs.qis[l]); + }
🧹 Nitpick comments (1)
circuits/lib/src/core/trbfv_pk.nr (1)
195-221: Reduce per-modulus evaluations to avoid O(L²) constraint blow‑up.
verify_public_key_for_modulusevaluates every polynomial for all CRT bases, but only indexiis used. Evaluating just theith polynomials reduces constraints without changing semantics.♻️ Suggested refactor
- let a_at_gamma = self.a.map(|a_poly| a_poly.eval(gamma)); - - let eek_at_gamma = self.eek.eval(gamma); - let sk_at_gamma = self.sk.eval(gamma); - - let r1_at_gamma = self.r1.map(|r1_poly| r1_poly.eval(gamma)); - let r2_at_gamma = self.r2.map(|r2_poly| r2_poly.eval(gamma)); - - let pk0_at_gamma = self.pk0.map(|pk0_poly| pk0_poly.eval(gamma)); - let pk1_at_gamma = self.pk1.map(|pk1_poly| pk1_poly.eval(gamma)); + let a_at_gamma = self.a[i].eval(gamma); + let eek_at_gamma = self.eek.eval(gamma); + let sk_at_gamma = self.sk.eval(gamma); + let r1_at_gamma = self.r1[i].eval(gamma); + let r2_at_gamma = self.r2[i].eval(gamma); + let pk0_at_gamma = self.pk0[i].eval(gamma); + let pk1_at_gamma = self.pk1[i].eval(gamma); @@ - let expected_pk0 = -a_at_gamma[i] * sk_at_gamma + let expected_pk0 = -a_at_gamma * sk_at_gamma + eek_at_gamma - + r2_at_gamma[i] * cyclo_at_gamma - + r1_at_gamma[i] * self.configs.qis[i]; + + r2_at_gamma * cyclo_at_gamma + + r1_at_gamma * self.configs.qis[i]; @@ - assert(pk0_at_gamma[i] == expected_pk0, "Public key equation 1 failed"); + assert(pk0_at_gamma == expected_pk0, "Public key equation 1 failed"); @@ - assert(pk1_at_gamma[i] == a_at_gamma[i], "Public key equation 2 failed"); + assert(pk1_at_gamma == a_at_gamma, "Public key equation 2 failed");
0xjei
approved these changes
Jan 20, 2026
Contributor
|
@cedoor get a look at my latest changes and, whenever we get green light from @zahrajavar is ready to be merged! |
0xjei
approved these changes
Jan 21, 2026
ctrlc03
approved these changes
Jan 21, 2026
This was referenced Jan 22, 2026
This was referenced Jan 30, 2026
This was referenced Feb 10, 2026
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
3 participants
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.
Closes #647
Summary by CodeRabbit
Release Notes
New Features
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.