refactor: recursion proof crisp [skip-line-limit]#1208
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSplits the CRISP circuit into CRISP and GRECO binaries, introducing GRECO-based BFV verification and ciphertext commitments. Refactors the circuits library, updates SDK APIs/types (BFV keypair, decrypt), swaps verifier calls to non-ZK honk, bumps tooling/dependencies, and adjusts build scripts and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GRECO
participant CRISP
participant Prover
participant Verifier
Client->>GRECO: submit BFV encryption polynomials + pk_commitment
GRECO->>GRECO: execute UserDataEncryption, compute k1_commitment & ct_commitment
GRECO-->>Client: return k1_commitment, ct_commitment, greco_proof
Client->>CRISP: submit vote inputs, merkle proof, greco_proof, commitments
CRISP->>CRISP: verify merkle proof, verify_honk_proof_non_zk(greco_proof), check commitments & signature
CRISP-->>Client: ct_commitment (or validation result)
Client->>Prover: request combined proof (CRISP + GRECO)
Prover->>Prover: generate recursive/combined proof
Prover-->>Client: proof
Client->>Verifier: submit proof
Verifier->>Verifier: verify combined proof
Verifier-->>Client: valid / invalid
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🧪 Generate unit tests (beta)
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
force-pushed
the
refactor/recursion-proof-crisp
branch
from
53c60e4 to
0ea4819
Compare
cedoor
force-pushed
the
refactor/recursion-proof-crisp
branch
from
0ea4819 to
16e995a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
examples/CRISP/crates/zk-inputs/src/ciphertext_addition.rs (1)
99-116:⚠️ Potential issue | 🟡 MinorStale doc: parameter
nis documented but doesn't exist in the signature.Line 110–111 documents a parameter
n(polynomial degree), but the function signature on lines 117–122 has no such parameter. This was likely left over from a previous version.📝 Suggested fix
/// # Arguments /// /// * `sum` - CRT polynomial of the sum ciphertext /// * `a` - CRT polynomial of the first ciphertext /// * `b` - CRT polynomial of the second ciphertext - /// * `n` - polynomial degree (number of coefficients per limb) /// * `moduli` - moduli for each CRT limbexamples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (1)
52-52:⚠️ Potential issue | 🔴 CriticalType mismatch:
voteusesbigint[]butVoteis nownumber[].
Votewas changed tonumber[]intypes.ts(line 70), but this test still uses bigint literals ([10n, 0n]). The same issue appears on line 111. This will cause TypeScript compilation errors and, at runtime, could produce incorrect circuit inputs since the encoding functions now expectnumbervalues.Proposed fix
- const vote = [10n, 0n] + const vote = [10, 0]And on line 111:
- const vote = [10n, 0n] + const vote = [10, 0]examples/CRISP/packages/crisp-sdk/src/types.ts (1)
82-110:⚠️ Potential issue | 🟡 MinorRemove unused type definitions
GrecoParams,GrecoCryptographicParams, andGrecoBoundParams.These types are declared but not referenced anywhere in the codebase. They were replaced by individual fields (
greco_verification_key,greco_key_hash,greco_proof, and commitment fields) inCRISPCircuitInputsand are now dead code that should be removed.examples/CRISP/scripts/compile_circuits.sh (1)
46-59:⚠️ Potential issue | 🟡 MinorFragile license header replacement — assumes exactly 2-line Apache preamble from
bb.
tail -n +3silently drops the first two lines. Ifbb write_solidity_verifierchanges its output format (e.g., adds a pragma or extra comment line), this will corrupt the file. Consider a more robust approach, such as stripping lines matching the Apache pattern.🛡️ More robust alternative
-# Remove the first 2 lines (Apache license and copyright) and prepend our license header -TEMP_FILE=$(mktemp) -{ - echo "$LICENSE_HEADER" - tail -n +3 packages/crisp-contracts/contracts/CRISPVerifier.sol -} > "$TEMP_FILE" -mv "$TEMP_FILE" packages/crisp-contracts/contracts/CRISPVerifier.sol +# Remove existing SPDX/Apache header lines and prepend our license header +TEMP_FILE=$(mktemp) +{ + echo "$LICENSE_HEADER" + # Skip lines matching Apache-2.0 license or copyright before the first code line + sed '1,/^\/\/ SPDX/d' packages/crisp-contracts/contracts/CRISPVerifier.sol || \ + tail -n +3 packages/crisp-contracts/contracts/CRISPVerifier.sol +} > "$TEMP_FILE" +mv "$TEMP_FILE" packages/crisp-contracts/contracts/CRISPVerifier.sol
🤖 Fix all issues with AI agents
In `@examples/CRISP/circuits/bin/crisp/Nargo.toml`:
- Line 9: The dependency entry bb_proof_verification = { git =
"https://github.com/AztecProtocol/aztec-packages/", tag =
"v3.0.0-nightly.20251104", directory = "barretenberg/noir/bb_proof_verification"
lacks an explanatory comment; add a short inline comment next to the
bb_proof_verification TOML line stating why this exact nightly tag is pinned
(e.g., required compatibility with Noir 1.0.0-beta.15, specific support for
non‑ZK recursive proofs, or coordination with a fixed commit that won’t be
rebased) so future maintainers understand the intentional choice. Ensure the
comment references the tag and the reason and stays brief.
In `@examples/CRISP/circuits/bin/crisp/src/main.nr`:
- Around line 55-57: The greco_key_hash input in main.nr is currently a private
Field allowing an attacker to substitute a malicious GRECO verification key;
change the declaration greco_key_hash: Field to greco_key_hash: pub Field (or
replace it with a hardcoded constant) so the value becomes a public input that
on-chain verifiers can check against the expected GRECO circuit hash; update
every occurrence of greco_key_hash (including the similar declarations around
lines 137-142) to use pub Field to ensure on-chain binding to the correct GRECO
verification key.
In `@examples/CRISP/circuits/bin/greco/Nargo.toml`:
- Around line 1-6: The package manifest's license field ("license = \"MIT\"" in
Nargo.toml) does not match the SPDX header in the source file
(SPDX-License-Identifier: LGPL-3.0-only in src/main.nr) — update one to make
them consistent; either change the Nargo.toml license value to "LGPL-3.0-only"
to match the SPDX header in src/main.nr (and the other example binaries/libs in
examples/CRISP/circuits), or update the SPDX header in each source file to "MIT"
so all SPDX headers and the Nargo.toml license field match across crisp_greco
and the sibling crisp binary and lib examples.
In `@examples/CRISP/circuits/lib/src/utils.nr`:
- Around line 27-31: The code computes segment_size = D / num_options even when
num_options may be 0; change both functions (the one setting let mut valid =
(num_options > 1) & (...) and check_coefficient_zero) to guard against
division-by-zero by early-returning false (or setting valid = false and
returning) before any division when num_options == 0 (or num_options <= 1), i.e.
check num_options first, then compute segment_size and the rest; reference the
symbols num_options, D, segment_size, MAX_OPTIONS, valid, and
check_coefficient_zero when making the change.
In `@examples/CRISP/crates/zk-inputs-wasm/src/lib.rs`:
- Around line 232-243: The current direct cast in decrypt_vote (in decrypt_vote
-> generator.decrypt_vote handling) uses "v as i64" which can overflow silently;
change the Vec<u64> -> Vec<i64> conversion to use TryFrom<i64> (or
i64::try_from) for each element, returning an Err(JsValue::from_str(...)) if any
conversion fails so failures are explicit; apply the same pattern in
encrypt_vote (and any other places doing "i64 as u64" or "u64 as i64") by using
TryFrom/try_into and mapping conversion errors to JsValue to fail gracefully
instead of wrapping.
In `@examples/CRISP/packages/crisp-sdk/src/utils.ts`:
- Around line 199-201: The helper bigInt64ArrayToNumberArray silently loses
precision by mapping BigInt values to Number; update bigInt64ArrayToNumberArray
to validate each element of the BigInt64Array against Number.MAX_SAFE_INTEGER
and Number.MIN_SAFE_INTEGER (or the equivalent negative bound) before
converting, and throw a clear RangeError (or return an explicit error) when any
value would lose precision; replace the Array.from(...).map(Number)
implementation with an explicit loop that checks each BigInt in the input and
only converts when safe to do so to prevent silent truncation.
- Around line 177-197: The function decodeBytesToNumbers currently reads u64
values with getBigUint64 but then converts them to Number, which silently loses
precision for values > Number.MAX_SAFE_INTEGER; update the implementation and
docs: either change the return type to bigint[] and keep returning the raw
BigInt array from decodeBytesToNumbers (update JSDoc to reflect bigint[]), or if
you must keep number[] add a runtime guard in decodeBytesToNumbers that checks
each BigInt against BigInt(Number.MAX_SAFE_INTEGER) and throws an Error when
exceeded (and update the JSDoc to state this runtime check), and ensure any
callers are adjusted accordingly (search references to decodeBytesToNumbers to
handle bigint[] or to catch the new error).
In `@examples/CRISP/packages/crisp-sdk/src/vote.ts`:
- Line 59: The computation of maxValue uses a 32-bit bit-shift causing overflow
for large MAX_VOTE_BITS; update the expression in vote.ts where maxValue is
computed (currently using "1 << maxBits") to use an exponentiation-based
calculation (2 ** maxBits - 1) so maxValue correctly represents a maxBits-wide
unsigned value and the subsequent bounds check (which uses maxValue and maxBits)
validates votes correctly.
🧹 Nitpick comments (10)
examples/CRISP/packages/crisp-sdk/package.json (1)
24-26: Consider parallel compilation for independent circuits.Since
compile:crispandcompile:grecoare independent of each other, you could run them in parallel to speed up the build:⚡ Suggested change
- "compile:circuits": "pnpm compile:crisp && pnpm compile:greco", + "compile:circuits": "pnpm compile:crisp & pnpm compile:greco & wait",examples/CRISP/crates/zk-inputs/src/lib.rs (1)
499-594: Solid test coverage for the newdecrypt_votemethod.The tests cover the essential scenarios: basic roundtrip, multiple vote patterns, wrong-key behavior, and invalid input handling. The wrong-key test at line 549 properly handles the non-deterministic BFV behavior (may succeed with garbage or may fail).
Minor nit: at lines 554 and 564,
_secret_key2uses the underscore-prefix convention (implying unused), but it is used on line 564. Consider renaming tosecret_key2.examples/CRISP/packages/crisp-sdk/src/types.ts (1)
156-158: Nit: Use/**for JSDoc consistency.Line 156 uses
/*instead of/**, unlike all other type doc comments in this file.-/* +/** * The inputs required for the GRECO circuit. */examples/CRISP/packages/crisp-sdk/src/vote.ts (2)
182-182:circuitInputs: anydefeats type safety across the proof pipeline.Both
generateCircuitInputs(line 182) andgenerateProof(line 258) useanyfor the circuit inputs, meaning any typo or missing field will only surface at circuit execution time. Consider defining a union or intersection type that captures the combined GRECO + CRISP fields returned by the WASM generator, so the compiler catches mismatches early.Also applies to: 258-258
62-63: Nit: dead branch —choiceIdx < vote.lengthis always true.The loop condition is
choiceIdx < nwheren = vote.length, so the ternary guard on line 63 can never take the0branch.- const value = choiceIdx < vote.length ? vote[choiceIdx] : 0 + const value = vote[choiceIdx]examples/CRISP/circuits/lib/src/ecdsa.nr (1)
155-181: Slightly misleading test comment.The comment on lines 157–159 says the test "will fail (assertion failure) when validate_signature is called with an invalid signature." Since
validate_signaturenow returnsfalserather than panicking, the actual failure comes fromassert(false)on line 180, not from withinvalidate_signatureitself. Consider updating the comment for clarity.📝 Suggested comment update
- // This test verifies that invalid signatures cause validation to fail. - // The test itself will fail (assertion failure) when validate_signature is called - // with an invalid signature, which demonstrates that invalid signatures are properly rejected. + // This test verifies that validate_signature returns false for an invalid signature. + // assert(false) triggers the expected panic, satisfying #[test(should_fail)].examples/CRISP/circuits/lib/src/commitments.nr (1)
38-41: Nit:payloaddoesn't needmutincompute_k1_commitment.Unlike
compute_ct_commitmentwherepayloadis reassigned on line 33, herepayloadis only assigned once on line 39.📝 Proposed fix
pub fn compute_k1_commitment<let N: u32, let BIT_K1: u32>(k1: Polynomial<N>) -> Field { - let mut payload = single_polynomial_payload::<N, BIT_K1>(Vec::new(), k1); + let payload = single_polynomial_payload::<N, BIT_K1>(Vec::new(), k1); compute_commitments(payload, DS_CRISP_K1, [0x80000000 | payload.len(), 1]).get(0) }examples/CRISP/scripts/compile_circuits.sh (1)
1-10: Script assumes a specific working directory.All paths are relative (e.g.,
circuits/bin/crisp), implying the script must be run fromexamples/CRISP/. Consider adding acdto the script's directory or documenting the expected invocation context.📝 Proposed addition at the top of the script
set -euo pipefail +# Ensure we're in the examples/CRISP directory +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cd "$SCRIPT_DIR/.." + echo "noir"examples/CRISP/circuits/lib/src/ciphertext_addition.nr (1)
151-160: Pre-existing:execute()mixes panic (range checks) with bool return (evaluations).
check_range_bounds()(line 153) asserts internally and will panic on failure, whileverify_evaluations()(line 159) returns a bool. This means callers get inconsistent failure modes — a panic for out-of-range coefficients vs.falsefor incorrect addition. Not introduced in this PR, but worth harmonizing as a follow-up if the broader refactor aims for consistent bool-based validation.examples/CRISP/circuits/bin/greco/src/main.nr (1)
34-51: The long generic parameter list is correct but consider a type alias for readability.The
UserDataEncryptioninstantiation with 12 generic parameters is faithful to the type definition but quite verbose. This is a minor readability concern — a type alias could make this more maintainable. That said, given this is a circuit entry point that's unlikely to be duplicated, this is acceptable as-is.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/CRISP/packages/crisp-sdk/src/vote.ts (2)
340-370:⚠️ Potential issue | 🟡 MinorType mismatch in
validateVote:reduceinitial value0vsbigintelements.Line 365:
vote.reduce((sum, v) => sum + v, 0)— ifVoteisbigint[], the initial value should be0n. With0(number), the first addition produces0 + bigintwhich coerces tobigintin JS but TypeScript will flag this as a type error with strict settings. Similarly, line 360's comparisonvotedAmount > balancemixesbigint | numberwithbigint.If
Voteis updated tonumber[](per the earlier comment), these become consistent. Otherwise, use0nand ensure consistent types.
92-123:⚠️ Potential issue | 🟡 Minor
decodeTallyreturnsVote(bigint[]) but pushesnumbervalues.Line 113 initializes
valueas0(number), and line 119 pushes it intoresults: Vote(which isbigint[]). This is a type inconsistency — the array will containnumbervalues at runtime while the type signature promisesbigint[].This is another symptom of the
Votetype needing to be updated tonumber[]to match the PR's direction.
🤖 Fix all issues with AI agents
In `@circuits/bin/recursive_aggregation/wrapper/dkg/share_computation/Nargo.toml`:
- Around line 5-9: Update the Nargo compiler version string in the Nargo.toml by
changing the compiler_version entry from "1.0.0" to "1.0.0-beta.18" so it
matches the NOIR_TOOLCHAIN used in CI; locate the compiler_version key in the
Nargo.toml (e.g., the line with compiler_version = "1.0.0") and replace its
value with "1.0.0-beta.18" across this file (and any other Nargo.toml files if
present) to ensure the declared toolchain matches the installed Nargo release.
In `@circuits/bin/threshold/user_data_encryption/Nargo.toml`:
- Line 5: The Nargo.toml files set compiler_version = "1.0.0" which mismatches
the CI NOIR_TOOLCHAIN pre-release v1.0.0-beta.18; update each Nargo.toml's
compiler_version entry to "1.0.0-beta.18" (or alternatively change the CI
NOIR_TOOLCHAIN to v1.0.0) so the nargo check uses the same toolchain; locate the
compiler_version key in the Nargo.toml files (e.g., the compiler_version field
in circuits/bin/threshold/user_data_encryption/Nargo.toml) and make the
consistent change across all 23 manifests.
In `@circuits/lib/src/math/safe.nr`:
- Line 346: The Vec::from_vector calls are inconsistent: one site uses
Vec::from_vector(&[1, 2, 3]) while other call sites use array literals directly;
standardize all Vec::from_vector invocations to the explicit slice form used in
the docs by changing the occurrences at the other call sites to use the &[...]
form (i.e., update each Vec::from_vector([...]) to Vec::from_vector(&[...]) so
all calls use the same & slice syntax).
In `@examples/CRISP/packages/crisp-sdk/src/utils.ts`:
- Around line 161-166: The getMaxVoteValue function uses a 32-bit left shift (1
<< effectiveBits) which overflows for effectiveBits > 31; update getMaxVoteValue
to compute the mask using an exponentiation-based or BigInt approach (e.g., use
2 ** effectiveBits - 1 or BigInt(1) << BigInt(effectiveBits) minus 1) instead of
the << operator so it returns correct values for up to MAX_VOTE_BITS; change the
expression in getMaxVoteValue accordingly.
In `@examples/CRISP/packages/crisp-sdk/src/vote.ts`:
- Around line 423-432: The verifyProof function currently instantiates
Barretenberg via Barretenberg.new and returns after calling
UltraHonkBackend.verifyProof but may skip api.destroy() if an exception occurs;
wrap the usage of the api and crispBackend.verifyProof call in a try/finally so
api.destroy() is always invoked (e.g., create api, then try { const isValid =
await crispBackend.verifyProof(...); return isValid } finally { api.destroy();
}), ensuring the cleanup runs even on errors while preserving the verifyProof
return value.
- Around line 259-333: The Barretenberg WASM instance created by
Barretenberg.new in generateProof can leak if any later step throws; wrap the
logic from after Barretenberg.new(...) through proof creation (everything that
uses the api, grecoBackend, crispBackend, etc.) in a try/finally block and call
api.destroy() in the finally so destroy runs on error or success, guarding the
destroy call so it only runs when api is defined; ensure generateProof still
rethrows errors after cleanup.
🧹 Nitpick comments (3)
examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
183-183:circuitInputs: anyloses type safety across the proof pipeline.
generateCircuitInputsreturnsany,generateProofacceptsany— property access typos or missing fields won't be caught at compile time. Consider defining a union or intersection type (e.g.,CRISPCircuitInputs & GrecoCircuitInputs & SignatureInputs) or at least aRecord<string, unknown>with runtime validation at the boundary.Also applies to: 188-188, 259-259
examples/CRISP/packages/crisp-sdk/src/utils.ts (1)
207-215: UsebytesToHexfrom viem instead ofBuffer.from().toString('hex')for browser compatibility.This SDK builds for ESM with bundler resolution (not Node.js-specific), so using
Bufferwithout a polyfill will cause runtime errors in browsers.bytesToHexfrom viem (already a project dependency) is already used in vote.ts for the same conversion and is the idiomatic solution here.Proposed fix
-import { publicKeyToAddress } from 'viem/utils' +import { publicKeyToAddress, bytesToHex } from 'viem/utils' import { hexToBytes, recoverPublicKey } from 'viem' export const proofToFields = (proof: Uint8Array): string[] => { const fields: string[] = [] for (let i = 0; i < proof.length; i += 32) { const chunk = proof.slice(i, i + 32) - fields.push('0x' + Buffer.from(chunk).toString('hex')) + fields.push(bytesToHex(chunk)) } return fields }examples/CRISP/circuits/lib/Nargo.toml (1)
9-11: Consider upgrading to newer compatible versions of external Noir dependencies.The current dependencies are compatible with Noir 1.0.0 (as indicated by the explicit
compiler_version = "1.0.0"setting in Nargo.toml and Noir tests in CI), but newer versions are available:
keccak256: v0.1.3 (currently v0.1.0)poseidon: v0.2.3 (currently v0.1.1; note v0.2.x may have API changes)binary_merkle_root: v0.0.1 (no newer version available)Evaluate upgrading
keccak256andposeidonto their latest patch/minor versions to benefit from potential improvements and bug fixes.
cedoor
force-pushed
the
refactor/recursion-proof-crisp
branch
from
9410d50 to
136e83f
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/CRISP/packages/crisp-sdk/src/types.ts (1)
82-110:⚠️ Potential issue | 🟡 MinorRemove unused type definitions
GrecoCryptographicParams,GrecoBoundParams, andGrecoParams.These types are not referenced anywhere in the codebase and have been replaced by the
GrecoCircuitInputstype introduced in the restructured circuit flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/CRISP/packages/crisp-sdk/src/types.ts` around lines 82 - 110, Remove the unused type definitions GrecoCryptographicParams, GrecoBoundParams, and GrecoParams from the file; delete their declarations and any related exports so the module only exposes currently used types (e.g., GrecoCircuitInputs) and avoid leaving unused imports/exports behind, then run TypeScript build/tsc to ensure no references remain and adjust any import sites if they accidentally referenced these types.
🧹 Nitpick comments (9)
packages/enclave-sdk/package.json (1)
53-53:vitestis listed underdependenciesinstead ofdevDependencies.This is a test runner and should be in
devDependenciesalongside the other dev tooling. This is pre-existing but worth cleaning up to avoid shipping unnecessary dependencies to consumers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/enclave-sdk/package.json` at line 53, The package.json currently lists the test runner "vitest" under dependencies; move the "vitest" entry from the top-level "dependencies" section into the "devDependencies" section in packages/enclave-sdk/package.json so it is not shipped to consumers, then update the lockfile by running the project's package manager install (e.g., npm/yarn/pnpm install) to reflect the change; ensure the unique key "vitest" is removed from "dependencies" and added under "devDependencies".examples/CRISP/crates/zk-inputs/src/lib.rs (1)
239-242: Custom secret key serialization bypassesSecretKey::from_bytes/SecretKeySerializer.The SK is serialized as raw
Vec<i64>coefficients viabincode::serialize(&sk.coeffs)and deserialized the same way. This is inconsistent with theSecretKey::from_bytes/SecretKeySerializerincrates/fhe/src/fhe.rs(which wraps inSecretKeyData).This works fine as long as
generate_keysanddecrypt_voteare always used together, but could cause confusing failures if keys are exchanged with code using the standard serialization format. Consider adding a doc comment noting the serialization format, or using the standardSecretKeySerializerfor interoperability.Also applies to: 263-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/CRISP/crates/zk-inputs/src/lib.rs` around lines 239 - 242, The code currently serializes/deserializes secret keys as raw Vec<i64> coefficients (using bincode) and constructs keys with SecretKey::new, which bypasses the standard SecretKey::from_bytes / SecretKeySerializer format and can break interoperability; fix by either (A) switching to the canonical serializer/deserializer (use SecretKeySerializer or call SecretKey::from_bytes/from_bytes(&SecretKeyData) when loading in the code paths around SecretKey::new, and update generate_keys/decrypt_vote to produce/consume that format), or (B) if keeping the raw-coeff format, add a clear doc comment on the functions (generate_keys, decrypt_vote and the load code around SecretKey::new) that states the exact serialization format and warns about incompatibility with SecretKeySerializer/SecretKey::from_bytes so callers know the contract.examples/CRISP/circuits/lib/src/commitments.nr (1)
38-41: Unnecessarymutonpayload.
payloadincompute_k1_commitmentis never reassigned after initialization (unlikecompute_ct_commitmentwhere it's reassigned on line 33). Themutqualifier can be removed.Proposed fix
pub fn compute_k1_commitment<let N: u32, let BIT_K1: u32>(k1: Polynomial<N>) -> Field { - let mut payload = single_polynomial_payload::<N, BIT_K1>(Vec::new(), k1); + let payload = single_polynomial_payload::<N, BIT_K1>(Vec::new(), k1); compute_commitments(payload, DS_CRISP_K1, [0x80000000 | payload.len(), 1]).get(0) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/CRISP/circuits/lib/src/commitments.nr` around lines 38 - 41, In compute_k1_commitment the local variable payload is declared mutable but never reassigned; remove the unnecessary mut qualifier from the payload binding in compute_k1_commitment (the call that constructs it uses single_polynomial_payload::<N, BIT_K1>(Vec::new(), k1)) so the variable is immutable, then keep the call to compute_commitments(payload, DS_CRISP_K1, [0x80000000 | payload.len(), 1]).get(0) unchanged.examples/CRISP/circuits/bin/crisp/src/main.nr (1)
181-195:CiphertextAddition::newandexecute()are computed unconditionally despite only being asserted in the mask-vote non-first-vote path.In Noir's constraint system, all computations contribute to the constraint count regardless of branching.
CiphertextAddition::newandexecute()will generate constraints even whenis_mask_vote == falseoris_first_vote == true. This is functionally correct (the result is only asserted conditionally), but it adds to the circuit's gate count — which is relevant given the PR's goal of reducing per-circuit constraints for browser WASM viability.Is there a way to defer or skip these constraints when they're not needed, or is this an acceptable cost given the current architecture?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/CRISP/circuits/bin/crisp/src/main.nr` around lines 181 - 195, CiphertextAddition::new and ct_add.execute() are always building constraints; only run them when needed by guarding creation/execution behind the runtime flags (is_mask_vote && !is_first_vote). Move the construction and execute call for ct_add/valid_ct_add inside the conditional that checks is_mask_vote and !is_first_vote, and in the else branch produce equivalent placeholder outputs (e.g., zero/false witnesses) so the rest of the circuit still composes; alternatively add a no-op constructor/factory on CiphertextAddition (e.g., CiphertextAddition::noop or CiphertextAddition::empty) that returns a structure that does not allocate constraints and whose execute() returns the placeholder result, then replace unconditional uses with the guarded/new-noop flow.examples/CRISP/packages/crisp-sdk/src/types.ts (1)
156-175: JSDoc comment uses/*instead of/**.The comment block for
GrecoCircuitInputsuses a single-star opening (/*) while other type comments in this file use the JSDoc convention (/**). This prevents documentation tools from picking it up.Suggested fix
-/* +/** * The inputs required for the GRECO circuit. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/CRISP/packages/crisp-sdk/src/types.ts` around lines 156 - 175, The comment block above the GrecoCircuitInputs type is using a plain block comment (/*) instead of a JSDoc comment, so documentation tools will skip it; update the comment to use the JSDoc opening /** (i.e., change the leading /* to /**) and ensure the comment text remains the same so GrecoCircuitInputs is picked up by docs generators and matches the other type comments in the file.examples/CRISP/packages/crisp-sdk/src/vote.ts (4)
183-231:circuitInputstyped asanyloses type safety across the proof generation pipeline.
generateCircuitInputsreturnscircuitInputs: any(line 183, 188), andgenerateProofacceptscircuitInputs: any(line 259). This means there's no compile-time verification that the WASM generator produces the correct shape or thatgenerateProofreceives all required fields (e.g.,pk0is,r1is,e0, etc. for GRECO).Consider defining a combined input type (or intersection of
CRISPCircuitInputsandGrecoCircuitInputsplus any additional fields from the WASM generator) to catch mismatches at compile time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/CRISP/packages/crisp-sdk/src/vote.ts` around lines 183 - 231, generateCircuitInputs currently returns circuitInputs: any (and generateProof accepts any), losing type safety; define a concrete combined type (e.g., type CombinedCircuitInputs = CRISPCircuitInputs & GrecoCircuitInputs & WasmGeneratedFields) that includes all expected fields from the WASM generator (pk0is, r1is, e0, etc.) and use it as the return type of generateCircuitInputs and the parameter type for generateProof; update occurrences of circuitInputs assignment in generateCircuitInputs to satisfy the new type (convert array/string mappings to the typed fields) and adjust any downstream consumers to rely on the new CombinedCircuitInputs interface so missing/incorrect fields are caught at compile time.
440-449: Verify public input indices inencodeSolidityProofmatch the updated circuit.
publicInputs[3]is assumed to beslot_addressandpublicInputs[6]is the return value (encrypted vote commitment). Based on the circuit's pub parameter order:prev_ct_commitment(0),pk_commitment(1),merkle_root(2),slot_address(3),is_first_vote(4),num_options(5), return(6) — this appears correct. However, any future reordering of pub inputs in the circuit would silently break this.Consider documenting the expected public input layout or using named constants for these indices.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/CRISP/packages/crisp-sdk/src/vote.ts` around lines 440 - 449, The function encodeSolidityProof uses hard-coded publicInputs indices (publicInputs[3] and publicInputs[6]) which can silently break if the circuit's public input order changes; update encodeSolidityProof to replace magic indices with descriptive constants (e.g., SLOT_ADDRESS_INDEX, RETURN_INDEX) or a small mapped accessor (e.g., getPubInput(publicInputs, 'slot_address')) and add a brief comment documenting the expected public input layout (prev_ct_commitment, pk_commitment, merkle_root, slot_address, is_first_vote, num_options, return) so future circuit reorderings are obvious and maintainers can update the constants instead of hunting for numeric indices.
246-252:returnValue as any[]cast is fragile — consider narrowing.The Noir
executemethod returns a generic type. Casting throughany[]silently swallows type mismatches. If the GRECO circuit's return type changes (e.g., returns a single value instead of a tuple), this will fail at runtime with an unhelpful error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/CRISP/packages/crisp-sdk/src/vote.ts` around lines 246 - 252, The cast "returnValue as any[]" in executeGrecoCircuit is fragile and can hide mismatches from noir.execute; update executeGrecoCircuit to narrow and validate returnValue before mapping: use a type guard or runtime check on the value returned by noir.execute (referencing noir.execute and the local variable returnValue) to confirm it's an array of hex strings (or handle the single-value case), throw a clear error if it’s not the expected shape, and then safely map each element to BigInt; this ensures changes to the GRECO circuit return type are caught and handled instead of failing silently at runtime.
340-369: Mixednumber > bigintcomparisons invalidateVoterely on implicit JS coercion.Lines 360 and 366 compare
number(fromvote[i]) againstbigint(balance). JavaScript supports relational operators across these types, but arithmetic mixing throws. This is subtle and could break if someone later refactors to use arithmetic (e.g.,balance - total).Consider explicitly converting for clarity:
Suggested fix
- const votedAmount = vote.find((v) => v > 0) ?? 0 - if (votedAmount > balance) { + const votedAmount = vote.find((v) => v > 0) ?? 0 + if (BigInt(votedAmount) > balance) { throw new Error('Invalid vote: vote exceeds balance') } } else { // 3+ options: split allowed, total capped - const total = vote.reduce((sum, v) => sum + v, 0) - if (total > balance) { + const total = vote.reduce((sum, v) => sum + v, 0) + if (BigInt(total) > balance) { throw new Error(`Invalid vote: total votes (${total}) exceed balance (${balance})`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/CRISP/packages/crisp-sdk/src/vote.ts` around lines 340 - 369, validateVote mixes numeric vote entries with a bigint balance causing subtle type-coercion risks; convert balance to a numeric value up-front (e.g., const balanceNum = Number(balance)) and use balanceNum for all comparisons and error messages involving vote values/totals (replace checks that use balance and interpolations that show balance with balanceNum). Update references in validateVote for the votedAmount > balance and total > balance checks (and any other comparisons against balance) to use balanceNum so arithmetic and comparisons remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Line 24: CI is pinning NOIR_TOOLCHAIN to v1.0.0-beta.18 while all Nargo.toml
files require compiler_version = "1.0.0", causing nargo check/compile failures;
update every Nargo.toml to set compiler_version = "1.0.0-beta.18" so it matches
the NOIR_TOOLCHAIN variable, verifying changes touch the compiler_version key in
each Nargo.toml and rerun CI locally to confirm nargo check passes.
In `@examples/CRISP/circuits/lib/Nargo.toml`:
- Line 8: The dependency name in the Nargo manifest is mismatched: the root file
declares enclave_lib in the dependency block but the target package's Nargo.toml
defines name = "lib"; fix by making these identifiers match — either change the
dependency key enclave_lib to lib in the dependency entry or rename the package
in circuits/lib/Nargo.toml to name = "enclave_lib" so the dependency name and
the package name are identical.
In `@examples/CRISP/packages/crisp-sdk/src/types.ts`:
- Line 70: Change the client vote type and vote construction to use number[]
instead of bigint[]: update the exported type Vote in
client/src/model/vote.model.ts (replace any "export type Vote = bigint[]" with
"export type Vote = number[]") and adjust the vote creation in
client/src/hooks/voting/useVoteCasting.ts where the variable vote is built
(replace BigInt literals like [balance, 0n] / [0n, balance] with numeric arrays
using Number(balance) or numeric literals, e.g., [Number(balance), 0] and [0,
Number(balance)]). Ensure all usages and imports referencing Vote reflect the
new number[] type.
In `@examples/CRISP/packages/crisp-sdk/src/utils.ts`:
- Around line 207-215: The proofToFields function currently calls
Buffer.from(...) which breaks in browsers; replace the Buffer usage with a
pure-TS/Web-API hex encoder: for each 32-byte chunk in proof (inside
proofToFields) convert the Uint8Array bytes to a hex string by mapping/iterating
bytes and producing two-digit hex (pad with '0' when needed) then prefix with
'0x' and push into fields; optionally factor the logic into a small helper like
byteArrayToHex to keep proofToFields concise and ensure it works in both Node
and browser environments.
In `@examples/CRISP/packages/crisp-sdk/tests/vote.test.ts`:
- Around line 219-221: The test is decrypting the local previousCiphertext
instead of the proof output; update the assertion to decrypt proof.encryptedVote
by calling decryptVote(proof.encryptedVote, secretKey, 2) and compare that
result to zeroVote so the test actually validates the proof's encryptedVote
rather than the trivially-initialized previousCiphertext.
---
Outside diff comments:
In `@examples/CRISP/packages/crisp-sdk/src/types.ts`:
- Around line 82-110: Remove the unused type definitions
GrecoCryptographicParams, GrecoBoundParams, and GrecoParams from the file;
delete their declarations and any related exports so the module only exposes
currently used types (e.g., GrecoCircuitInputs) and avoid leaving unused
imports/exports behind, then run TypeScript build/tsc to ensure no references
remain and adjust any import sites if they accidentally referenced these types.
---
Duplicate comments:
In `@circuits/bin/config/Nargo.toml`:
- Line 5: The compiler_version value in Nargo.toml is inconsistent with the CI
toolchain; update the compiler_version setting (the compiler_version entry in
Nargo.toml) to match the CI's version string (v1.0.0-beta.18) so both the
repository and CI use the exact same compiler version string.
In `@circuits/bin/dkg/pk/Nargo.toml`:
- Line 5: The compiler_version field in Nargo.toml currently reads "1.0.0" and
must be updated to match the CI toolchain; open the Nargo.toml file and change
the compiler_version value to the exact version used by CI (replace "1.0.0" with
the CI toolchain version), saving the file so the build uses the correct
compiler version.
In `@circuits/bin/dkg/share_encryption/Nargo.toml`:
- Line 5: The Nargo.toml contains a mismatched compiler_version entry; update
the compiler_version value in Nargo.toml to match the expected version used in
CI (the same version referenced in ci.yml line 24) so both places agree; locate
the compiler_version key in Nargo.toml and replace its current string with the
CI's canonical compiler version.
In `@circuits/bin/recursive_aggregation/fold/Nargo.toml`:
- Line 5: The Nargo.toml contains compiler_version = "1.0.0" which mismatches
the CI toolchain; update the compiler_version entry to the exact compiler
version string used by CI (replace "1.0.0" with the CI toolchain's compiler
version) so the compiler_version field matches the CI toolchain.
In `@circuits/bin/recursive_aggregation/wrapper/dkg/share_computation/Nargo.toml`:
- Line 5: The compiler_version field in Nargo.toml is incorrect; update the
compiler_version value (the compiler_version key) to match the
expected/consistent version used across the project (replace "1.0.0" with the
canonical compiler version used elsewhere) so all Nargo.toml files use the same
compiler_version string.
In `@circuits/bin/recursive_aggregation/wrapper/dkg/share_decryption/Nargo.toml`:
- Line 5: The Nargo.toml currently sets compiler_version = "1.0.0" which
mismatches the CI toolchain; update the compiler_version entry in Nargo.toml to
the CI-required version string (replace the "1.0.0" value) so it matches the
toolchain used in CI (ensure the compiler_version key remains unchanged and only
the value is updated).
In
`@circuits/bin/recursive_aggregation/wrapper/threshold/pk_generation/Nargo.toml`:
- Line 5: The project's Nargo.toml has an outdated compiler_version entry
("compiler_version = \"1.0.0\""); update the compiler_version field in
Nargo.toml to match the CI toolchain's required version (replace the current
value with the CI's compiler version string) so the build toolchain aligns with
CI; locate the compiler_version key in Nargo.toml and change its value
accordingly.
In
`@circuits/bin/recursive_aggregation/wrapper/threshold/share_decryption/Nargo.toml`:
- Line 5: compiler_version in Nargo.toml ("compiler_version = \"1.0.0\"")
mismatches the CI-declared compiler version; update the compiler_version key in
Nargo.toml to the exact same version string used by the project's CI
configuration (the canonical compiler_version) so both are identical and keep
them synchronized going forward.
In `@circuits/bin/threshold/decrypted_shares_aggregation_mod/Nargo.toml`:
- Line 5: The compiler_version entry in Nargo.toml (the compiler_version =
"1.0.0" line) is out of sync with the CI toolchain; update the compiler_version
value to the correct version used by CI (match the exact string used elsewhere
in the repo), and ensure other Nargo.toml files use the same version to keep
consistency across the project.
In `@circuits/bin/threshold/pk_aggregation/Nargo.toml`:
- Line 5: The compiler_version setting in Nargo.toml ("compiler_version") is
inconsistent with the version used in CI; update the compiler_version value in
the pk_aggregation/Nargo.toml (symbol: compiler_version) to match the canonical
compiler version used in the CI configuration (the same version string
referenced in ci.yml) so both files use the identical compiler version.
In `@circuits/bin/threshold/pk_generation/Nargo.toml`:
- Line 5: The TOML key compiler_version in Nargo.toml is inconsistent with the
CI config; update the compiler_version value to exactly match the compiler
version specified in the CI configuration (the value used at ci.yml line 24) so
both places use the same version string.
In `@circuits/bin/threshold/user_data_encryption/Nargo.toml`:
- Line 5: The Nargo.toml has a mismatched compiler_version value; update the
compiler_version entry in Nargo.toml to the canonical compiler version used by
the project (the same value declared in CI) so they match; locate the
compiler_version key in Nargo.toml and change its string to the project's
canonical compiler version.
In `@circuits/lib/Nargo.toml`:
- Line 5: The compiler_version field in Nargo.toml is set to "1.0.0" which will
mismatch the CI-installed Nargo toolchain; update the compiler_version value in
Nargo.toml to the exact version pinned by CI (e.g., "1.0.0-beta.18") so it
matches NOIR_TOOLCHAIN/Nargo, ensuring Nargo's version check passes during
builds.
In `@examples/CRISP/circuits/bin/crisp/src/main.nr`:
- Around line 55-63: The greco_key_hash field is currently private which lets a
prover substitute an arbitrary GRECO verification key; change the declaration of
greco_key_hash to be public (greco_key_hash: pub Field) or replace it with a
hardcoded constant bound to the expected GRECO VK so the on-chain verifier can
trust the GRECO VK; ensure references in verification logic that rely on
greco_key_hash (and related symbols UltraHonkVerificationKey, greco_proof,
k1_commitment, ct_commitment) use the public/hardcoded value so the re-derived
k1_commitment and ct_commitment cannot be forged by a malicious prover.
In `@examples/CRISP/crates/zk-inputs-wasm/src/lib.rs`:
- Around line 232-243: The current cast in decrypt_vote (function decrypt_vote
calling self.generator.decrypt_vote) uses v as i64 which silently wraps for u64
≥ 2^63; change the JS-facing API to avoid lossy casts by returning JS-friendly
values: map each u64 to a JsValue and for values > i64::MAX create a
js_sys::BigInt (js_sys::BigInt::from(u64)) and for smaller values use
JsValue::from_f64(v as f64) or JsValue::from(i64) so no wrap occurs; update the
function signature to return Result<Vec<JsValue>, JsValue> and adjust callers
accordingly (preserve error handling that converts e.to_string()).
In `@examples/CRISP/packages/crisp-sdk/src/utils.ts`:
- Around line 177-197: The function decodeBytesToNumbers currently collects
BigInts via DataView.getBigUint64 but then silently converts them to JS Numbers
with result.map(Number), which loses precision; update the implementation and
signature so it returns bigint[] (remove the final .map(Number) and return
result directly), and update the JSDoc to state it returns bigint[] (or,
alternatively, if a number[] is required, add an explicit check in
decodeBytesToNumbers that throws an error when any value exceeds
Number.MAX_SAFE_INTEGER before converting); reference the function name
decodeBytesToNumbers and the use of DataView.getBigUint64/result.map(Number)
when making the change.
- Around line 161-166: getMaxVoteValue uses bit-shift (1 << effectiveBits) which
overflows for effectiveBits > 31; replace that expression with an
exponentiation-based computation such as (2 ** effectiveBits) - 1 (or
Math.pow(2, effectiveBits) - 1) in getMaxVoteValue so effectiveBits up to
MAX_VOTE_BITS (50) are handled correctly and the function still returns a
number; keep the rest of the logic (bfvParams.degree, segmentSize,
effectiveBits) unchanged.
In `@examples/CRISP/packages/crisp-sdk/src/vote.ts`:
- Around line 423-432: The verifyProof function leaks the Barretenberg API when
crispBackend.verifyProof throws because api.destroy() is called unconditionally
after the await; wrap the usage of the Barretenberg API in a try/finally: create
the API via Barretenberg.new, construct UltraHonkBackend(crispCircuit.bytecode,
api), then call crispBackend.verifyProof inside the try block and always call
api.destroy() in the finally block so the API is cleaned up on success or error.
- Around line 259-333: generateProof leaks the Barretenberg WASM instance
because api.destroy() is only called on the happy path; wrap creation/use of the
Barretenberg API in a try/finally so api.destroy() runs even on exceptions.
Specifically, after calling Barretenberg.new(...) (in generateProof) ensure you
assign the api variable and then place all uses of api (UltraHonkBackend
construction, generateProof, generateRecursiveProofArtifacts, generateProof for
crisp) inside a try block and call api.destroy() in the finally block; keep the
existing return of the proof but rethrow any caught error if needed so behavior
is preserved.
---
Nitpick comments:
In `@examples/CRISP/circuits/bin/crisp/src/main.nr`:
- Around line 181-195: CiphertextAddition::new and ct_add.execute() are always
building constraints; only run them when needed by guarding creation/execution
behind the runtime flags (is_mask_vote && !is_first_vote). Move the construction
and execute call for ct_add/valid_ct_add inside the conditional that checks
is_mask_vote and !is_first_vote, and in the else branch produce equivalent
placeholder outputs (e.g., zero/false witnesses) so the rest of the circuit
still composes; alternatively add a no-op constructor/factory on
CiphertextAddition (e.g., CiphertextAddition::noop or CiphertextAddition::empty)
that returns a structure that does not allocate constraints and whose execute()
returns the placeholder result, then replace unconditional uses with the
guarded/new-noop flow.
In `@examples/CRISP/circuits/lib/src/commitments.nr`:
- Around line 38-41: In compute_k1_commitment the local variable payload is
declared mutable but never reassigned; remove the unnecessary mut qualifier from
the payload binding in compute_k1_commitment (the call that constructs it uses
single_polynomial_payload::<N, BIT_K1>(Vec::new(), k1)) so the variable is
immutable, then keep the call to compute_commitments(payload, DS_CRISP_K1,
[0x80000000 | payload.len(), 1]).get(0) unchanged.
In `@examples/CRISP/crates/zk-inputs/src/lib.rs`:
- Around line 239-242: The code currently serializes/deserializes secret keys as
raw Vec<i64> coefficients (using bincode) and constructs keys with
SecretKey::new, which bypasses the standard SecretKey::from_bytes /
SecretKeySerializer format and can break interoperability; fix by either (A)
switching to the canonical serializer/deserializer (use SecretKeySerializer or
call SecretKey::from_bytes/from_bytes(&SecretKeyData) when loading in the code
paths around SecretKey::new, and update generate_keys/decrypt_vote to
produce/consume that format), or (B) if keeping the raw-coeff format, add a
clear doc comment on the functions (generate_keys, decrypt_vote and the load
code around SecretKey::new) that states the exact serialization format and warns
about incompatibility with SecretKeySerializer/SecretKey::from_bytes so callers
know the contract.
In `@examples/CRISP/packages/crisp-sdk/src/types.ts`:
- Around line 156-175: The comment block above the GrecoCircuitInputs type is
using a plain block comment (/*) instead of a JSDoc comment, so documentation
tools will skip it; update the comment to use the JSDoc opening /** (i.e.,
change the leading /* to /**) and ensure the comment text remains the same so
GrecoCircuitInputs is picked up by docs generators and matches the other type
comments in the file.
In `@examples/CRISP/packages/crisp-sdk/src/vote.ts`:
- Around line 183-231: generateCircuitInputs currently returns circuitInputs:
any (and generateProof accepts any), losing type safety; define a concrete
combined type (e.g., type CombinedCircuitInputs = CRISPCircuitInputs &
GrecoCircuitInputs & WasmGeneratedFields) that includes all expected fields from
the WASM generator (pk0is, r1is, e0, etc.) and use it as the return type of
generateCircuitInputs and the parameter type for generateProof; update
occurrences of circuitInputs assignment in generateCircuitInputs to satisfy the
new type (convert array/string mappings to the typed fields) and adjust any
downstream consumers to rely on the new CombinedCircuitInputs interface so
missing/incorrect fields are caught at compile time.
- Around line 440-449: The function encodeSolidityProof uses hard-coded
publicInputs indices (publicInputs[3] and publicInputs[6]) which can silently
break if the circuit's public input order changes; update encodeSolidityProof to
replace magic indices with descriptive constants (e.g., SLOT_ADDRESS_INDEX,
RETURN_INDEX) or a small mapped accessor (e.g., getPubInput(publicInputs,
'slot_address')) and add a brief comment documenting the expected public input
layout (prev_ct_commitment, pk_commitment, merkle_root, slot_address,
is_first_vote, num_options, return) so future circuit reorderings are obvious
and maintainers can update the constants instead of hunting for numeric indices.
- Around line 246-252: The cast "returnValue as any[]" in executeGrecoCircuit is
fragile and can hide mismatches from noir.execute; update executeGrecoCircuit to
narrow and validate returnValue before mapping: use a type guard or runtime
check on the value returned by noir.execute (referencing noir.execute and the
local variable returnValue) to confirm it's an array of hex strings (or handle
the single-value case), throw a clear error if it’s not the expected shape, and
then safely map each element to BigInt; this ensures changes to the GRECO
circuit return type are caught and handled instead of failing silently at
runtime.
- Around line 340-369: validateVote mixes numeric vote entries with a bigint
balance causing subtle type-coercion risks; convert balance to a numeric value
up-front (e.g., const balanceNum = Number(balance)) and use balanceNum for all
comparisons and error messages involving vote values/totals (replace checks that
use balance and interpolations that show balance with balanceNum). Update
references in validateVote for the votedAmount > balance and total > balance
checks (and any other comparisons against balance) to use balanceNum so
arithmetic and comparisons remain consistent.
In `@packages/enclave-sdk/package.json`:
- Line 53: The package.json currently lists the test runner "vitest" under
dependencies; move the "vitest" entry from the top-level "dependencies" section
into the "devDependencies" section in packages/enclave-sdk/package.json so it is
not shipped to consumers, then update the lockfile by running the project's
package manager install (e.g., npm/yarn/pnpm install) to reflect the change;
ensure the unique key "vitest" is removed from "dependencies" and added under
"devDependencies".
- Updated @crisp-e3/sdk to 0.6.0 - Updated @crisp-e3/contracts to 0.6.0 - Updated @crisp-e3/zk-inputs to 0.6.0 - Published to npm
- Use chain block timestamp in CountdownTime so countdown matches on-chain end (block.timestamp > end_time) instead of local Date.now() - Compute voting window from RPC timestamp in CLI and HTTP init with 20s buffer for tx mining before window opens; window_end = start + e3_duration - Replace chrono Utc::now() with get_current_timestamp_rpc() in rounds route - Make E2E WAIT derive from E3_DURATION env var instead of hardcoded 280s - Reduce E3_DURATION in .env.example from 250 to 80 for faster iteration - Load server .env in crisp.spec.ts for dynamic config in tests
|
it passes 😭 |
5 participants
The CRISP circuit with production params had ~6M gates, which made it impractical to generate proofs in browsers or Node.js due to memory and time limits.
Solution: Sub-circuit Splitting
The monolithic circuit is now split into several sub-circuits:
The final on-chain proof is produced by the fold circuit.
Toolchain Updates
New BB supports noir-recursive-no-zk for non-zk proofs (reduced recursion overhead)
CRISP SDK Updates
Results
Proof generation for the new circuits takes around 208 seconds with secure parameters.
Diagram
Closes #1201