fix: crisp circuit validation of encoded vote#973
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughTightens an assertion in the Noir main entry, reverses coefficient bit-processing and validation in Noir utils, adjusts the SDK maximum vote constant, adds an SDK helper to fetch circuit return values, and updates tests and test utilities accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Tests
participant SDK as crisp-sdk
participant Circuit as CRISP Circuit
Note over Test,SDK: Requesting circuit output and proof
Test->>SDK: call getCircuitOutputValue(crispInputs)
SDK->>Circuit: invoke circuit with inputs
alt circuit succeeds
Circuit-->>SDK: returnValue
SDK-->>Test: Promise resolves { returnValue }
else assertion or validation fails
Circuit-->>SDK: assertion/error
SDK-->>Test: Promise rejects with error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
100-130: LGTM - Improved bit extraction logic.The refactored
decodeTallyfunction now uses proper index boundaries and bit-level weight calculations instead of Horner-style accumulation. The logic correctly extracts the relevant portions of the tally based onHALF_LARGEST_MINIMUM_DEGREEand converts them using binary weights.However, wrap the switch case declarations in a block to prevent scope leakage:
switch (votingMode) { - case VotingMode.GOVERNANCE: + case VotingMode.GOVERNANCE: { const HALF_D = tally.length / 2 const START_INDEX_Y = HALF_D - HALF_LARGEST_MINIMUM_DEGREE const START_INDEX_N = tally.length - HALF_LARGEST_MINIMUM_DEGREE // Extract only the relevant parts of the tally const yesBinary = tally.slice(START_INDEX_Y, HALF_D) const noBinary = tally.slice(START_INDEX_N, tally.length) let yes = 0n let no = 0n // Convert yes votes (from START_INDEX_Y to HALF_D) for (let i = 0; i < yesBinary.length; i += 1) { const weight = 2n ** BigInt(yesBinary.length - 1 - i) yes += BigInt(yesBinary[i]) * weight } // Convert no votes (from START_INDEX_N to D) for (let i = 0; i < noBinary.length; i += 1) { const weight = 2n ** BigInt(noBinary.length - 1 - i) no += BigInt(noBinary[i]) * weight } return { yes, no, } + } default: throw new Error('Unsupported voting mode') }As per static analysis hints.
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (1)
382-383: Complete or remove placeholder tests.Two test cases are defined but not implemented. Please either implement these tests or remove them to avoid confusion.
Do you want me to help generate the implementation for these test cases, or would you like to track them in a separate issue?
examples/CRISP/circuits/src/utils.nr (2)
19-23: Clarify the "reversal" terminology in comments.The comments reference "after reversal" and describe bits moving positions, but there is no actual reversal operation in the code. These comments appear to describe the semantic interpretation of bit positions rather than an algorithmic step. Consider rephrasing to avoid implying an operation that doesn't exist, e.g., "The END indices mark where we stop reading bits" or "Bits at higher indices represent more significant bit positions."
28-31: Consider explicitly defining START_INDEX_Y for clarity.The comment mentions "from END to START" but
START_INDEX_Yis not defined in this function (though it's implicitlyHALF_D). While the logic is correct, explicitly definingSTART_INDEX_Y = HALF_Dwould improve readability and make the windowing boundaries more obvious, similar to how it's done incheck_coefficient_zeroat lines 64-68.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/CRISP/circuits/src/main.nr(1 hunks)examples/CRISP/circuits/src/utils.nr(8 hunks)examples/CRISP/packages/crisp-sdk/src/constants.ts(1 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(3 hunks)examples/CRISP/packages/crisp-sdk/tests/utils.ts(1 hunks)examples/CRISP/packages/crisp-sdk/tests/vote.test.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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]`.
📚 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:
examples/CRISP/circuits/src/main.nrexamples/CRISP/circuits/src/utils.nr
📚 Learning: 2025-08-27T14:02:25.412Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: examples/CRISP/circuits/src/main.nr:40-44
Timestamp: 2025-08-27T14:02:25.412Z
Learning: In CRISP circuits using Greco library, k1 represents a binary value encoded as a polynomial where only one specific coefficient (k1.coefficients[2048 - 1]) carries the actual binary information and needs the constraint `assert(0 == b * (qmt - b))`. Other coefficients in the polynomial are not significant and don't require the binary constraint.
Applied to files:
examples/CRISP/circuits/src/main.nrexamples/CRISP/circuits/src/utils.nr
📚 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.nr
🧬 Code graph analysis (2)
examples/CRISP/packages/crisp-sdk/src/vote.ts (2)
examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
HALF_LARGEST_MINIMUM_DEGREE(17-17)examples/CRISP/packages/crisp-sdk/src/types.ts (1)
CRISPCircuitInputs(135-173)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (3)
examples/CRISP/packages/crisp-sdk/src/vote.ts (5)
decodeTally(97-130)generateMaskVote(221-262)encodeVote(64-90)encryptVoteAndGenerateCRISPInputs(170-208)getCircuitOutputValue(286-292)examples/CRISP/packages/crisp-sdk/tests/utils.ts (1)
compareCoefficientsArrays(14-35)examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(18-20)generateMerkleProof(39-79)
🪛 Biome (2.1.2)
examples/CRISP/packages/crisp-sdk/src/vote.ts
[error] 100-100: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 101-101: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 102-102: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 105-105: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 106-106: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 108-108: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 109-109: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 Gitleaks (8.28.0)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts
[high] 356-356: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 390-390: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (8)
- GitHub Check: build_e3_support_dev
- GitHub Check: build_sdk
- GitHub Check: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: test_net
- GitHub Check: rust_unit
- GitHub Check: rust_integration
- GitHub Check: test_contracts
🔇 Additional comments (16)
examples/CRISP/circuits/src/main.nr (1)
123-123: LGTM - Consistent assertion style.The explicit boolean comparison aligns with the assertion style on line 113, improving consistency throughout the conditional logic.
examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
17-23: LGTM - Maximum vote value correctly reduced.The calculation now correctly produces 2^28 - 1 (268,435,455) as the maximum vote value, and the comment accurately reflects this change. This aligns with the PR objective of tightening coefficient validation.
examples/CRISP/packages/crisp-sdk/tests/utils.ts (1)
7-35: LGTM - Formatting improvements.The changes improve code consistency by removing unnecessary semicolons and tightening spacing. No functional changes to the utility functions.
examples/CRISP/packages/crisp-sdk/src/vote.ts (2)
274-284: LGTM - Clear signature formatting.The multi-line parameter formatting improves readability, and the return type annotation makes the function's contract explicit.
286-292: LGTM - Useful validation utility.The new
getCircuitOutputValuefunction provides a lightweight way to retrieve circuit output for validation without the overhead of proof generation. This is particularly useful for testing and error handling scenarios.examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (6)
17-17: LGTM - New function imported for testing.The import of
getCircuitOutputValueenables validation testing without proof generation overhead.
58-120: LGTM - Test data updated for new logic.The expanded test data and updated expectations (yes: 22n, no: 1n) correctly validate the refactored
decodeTallyfunction's bit extraction logic.
233-240: LGTM - Improved readability.The multi-line formatting makes the function arguments easier to review.
330-331: LGTM - Consistent assertion style.The assertions correctly use
compareCoefficientsArraysfor deep comparison of coefficient arrays.Also applies to: 348-350
352-381: LGTM - Good error handling coverage.The new test case validates that the circuit correctly rejects inputs with invalid signatures by using
getCircuitOutputValueand asserting rejection.
385-415: LGTM - Balance validation coverage.The test correctly verifies that the circuit rejects votes exceeding the voter's balance by setting balance to '0' and asserting rejection.
examples/CRISP/circuits/src/utils.nr (5)
33-33: Coefficient validation correctly enforces modular arithmetic constraint.The assertion
0 == coeff * (q_mod_t - coeff)properly constrains each coefficient to be either 0 orq_mod_t, replacing the previous binary (0 or 1) constraint with modular arithmetic. This is the correct way to validate coefficients in the CRISP encoding scheme.Also applies to: 43-43
35-36: Bit accumulation logic is correct.The bit extraction (
if coeff == 0 { 0 } else { 1 }) combined with the accumulation pattern (sum * 2 + bit) correctly reconstructs the binary value from the polynomial coefficients. Processing from higher to lower indices means that coefficients at higher indices contribute more significant bits, which aligns with the expected encoding.Also applies to: 45-46
60-85: Index range consistency verified.The ranges checked here (
START_INDEX_Y..END_INDEX_YandSTART_INDEX_N..END_INDEX_N) are consistent with the ranges processed incheck_coefficient_values_with_balance. Both functions operate on the same coefficient windows (indices 0-27 for NO votes, indices 50-77 for YES votes when D=100), ensuring validation consistency across the codebase.
87-190: Test cases correctly updated for new semantics.All test cases have been properly updated to reflect the new coefficient validation (0 or
q_mod_t) and reversed bit-processing order. The test data and expected outcomes align correctly with the modified validation logic:
- Bit positions at indices 50-52 correctly encode value 7
- Invalid coefficient value 2 (not 0 or 1 when
q_mod_t=1) correctly triggers assertion failure- Balance checks properly validate against reconstructed binary values
15-16: Maximum vote value calculation is correct.The value 268,435,455 correctly represents the maximum that can be encoded in 28 bits (2^28 - 1), which aligns with the PR objective of reducing the maximum vote value by one from the previous 2^28.
ctrlc03
force-pushed
the
fix/circuit-encoding-validation
branch
from
686efcf to
cd559e2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
examples/CRISP/circuits/src/utils.nr (1)
87-161: Update test comment to reflect current validation logic.The test data and structure look correct, but the comment on Line 152 still says "Invalid coefficient value 2 (not 0 or 1)" even though the validation now checks for
0 or q_mod_t.Apply this diff to update the comment:
- // Invalid coefficient value 2 (not 0 or 1) + // Invalid coefficient value 2 (not 0 or q_mod_t)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
examples/CRISP/circuits/src/main.nr(1 hunks)examples/CRISP/circuits/src/utils.nr(8 hunks)examples/CRISP/packages/crisp-sdk/src/constants.ts(1 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(3 hunks)examples/CRISP/packages/crisp-sdk/tests/utils.ts(1 hunks)examples/CRISP/packages/crisp-sdk/tests/vote.test.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/CRISP/circuits/src/main.nr
- examples/CRISP/packages/crisp-sdk/tests/utils.ts
🧰 Additional context used
🧠 Learnings (3)
📚 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:
examples/CRISP/circuits/src/utils.nr
📚 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.nr
📚 Learning: 2025-08-27T14:02:25.412Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: examples/CRISP/circuits/src/main.nr:40-44
Timestamp: 2025-08-27T14:02:25.412Z
Learning: In CRISP circuits using Greco library, k1 represents a binary value encoded as a polynomial where only one specific coefficient (k1.coefficients[2048 - 1]) carries the actual binary information and needs the constraint `assert(0 == b * (qmt - b))`. Other coefficients in the polynomial are not significant and don't require the binary constraint.
Applied to files:
examples/CRISP/circuits/src/utils.nr
🧬 Code graph analysis (2)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (5)
examples/CRISP/packages/crisp-sdk/src/vote.ts (5)
decodeTally(97-130)generateMaskVote(221-262)encodeVote(64-90)encryptVoteAndGenerateCRISPInputs(170-208)getCircuitOutputValue(286-292)examples/CRISP/packages/crisp-sdk/src/constants.ts (2)
DEFAULT_BFV_PARAMS(29-29)MESSAGE(34-34)examples/CRISP/packages/crisp-sdk/tests/constants.ts (6)
merkleProof(33-33)testAddress(32-32)VOTE(14-14)votingPowerLeaf(31-31)MESSAGE(11-11)LEAVES(16-27)examples/CRISP/packages/crisp-sdk/tests/utils.ts (1)
compareCoefficientsArrays(14-35)examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(18-20)generateMerkleProof(39-79)
examples/CRISP/packages/crisp-sdk/src/vote.ts (2)
examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
HALF_LARGEST_MINIMUM_DEGREE(17-17)examples/CRISP/packages/crisp-sdk/src/types.ts (1)
CRISPCircuitInputs(135-173)
🪛 Biome (2.1.2)
examples/CRISP/packages/crisp-sdk/src/vote.ts
[error] 100-100: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 101-101: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 102-102: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 105-105: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 106-106: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 108-108: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 109-109: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 Gitleaks (8.28.0)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts
[high] 356-356: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 390-390: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (8)
- GitHub Check: build_e3_support_dev
- GitHub Check: test_contracts
- GitHub Check: integration_prebuild
- GitHub Check: test_net
- GitHub Check: build_enclave_cli
- GitHub Check: build_sdk
- GitHub Check: rust_unit
- GitHub Check: rust_integration
🔇 Additional comments (13)
examples/CRISP/circuits/src/utils.nr (5)
9-16: LGTM: Assertion and bound updates align with coefficient reversal fix.The validation now checks coefficients are either 0 or
q_mod_t(instead of 0 or 1), and the maximum value correctly reflects 2^28 - 1 (268,435,455), consistent with the SDK constant changes.
18-37: LGTM: Yes-part reversal logic correctly implements MSB-first processing.The new END_INDEX-based approach processes coefficients in reverse order (from
HALF_D + 27down toHALF_D), and the bit extraction + accumulation (sum = sum * 2 + bit) correctly builds the decimal value from the reversed binary representation.
39-47: LGTM: No-part reversal logic mirrors yes-part correctly.The no-part processing uses the same reversed traversal pattern (indices 27 down to 0) with identical bit extraction and accumulation logic.
60-85: LGTM: Coefficient zero check ranges updated correctly.The function now checks the correct ranges (50..78 for yes, 0..28 for no) that correspond to the valid coefficient windows after the reversal changes.
163-190: LGTM: Zero coefficient tests updated correctly.The test data and comments correctly reflect the new indexing scheme for the yes section starting at index 50.
examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
17-23: LGTM: Maximum vote value correctly adjusted to 2^28 - 1.The change correctly reflects that 28 bits can represent values from 0 to 268,435,455 (2^28 - 1), aligning with the circuit's updated validation logic.
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (4)
17-17: LGTM: New helper function import for circuit execution testing.The
getCircuitOutputValuefunction enables testing circuit execution without proof generation, improving test performance.
233-240: LGTM: Formatting improvement for readability.Multi-line parameter formatting improves code readability with no behavioral changes.
352-381: LGTM: Invalid signature test correctly validates circuit rejection.The test appropriately invalidates the signature and verifies the circuit execution fails. The hardcoded private key is the Hardhat default test key, which is safe for testing.
Note: The static analysis warning about the hardcoded key on line 356 is a false positive—this is a well-known test key from Hardhat.
56-120: Expected tally decode values are correct.The test values (
yes: 22n,no: 1n) correctly correspond to the tally data when decoded with the GOVERNANCE mode logic. WithHALF_LARGEST_MINIMUM_DEGREE = 28and tally length 56:
yesBinaryextracts indices 0-27, containing5at index 25 and2at index 27, decoding to 22 (5×4 + 2×1)noBinaryextracts indices 28-55, containing1at index 55, decoding to 1examples/CRISP/packages/crisp-sdk/src/vote.ts (3)
97-130: LGTM: Decode logic formatting updated with no behavioral changes.The function correctly extracts and decodes the relevant coefficient ranges. Note that Biome's
noSwitchDeclarationswarnings are valid (switch case variables should be wrapped in blocks), but this is a pre-existing issue that can be addressed separately.
274-284: LGTM: Formatting improvement for function signature.Multi-line parameter formatting improves readability with no behavioral changes.
286-292: LGTM: New helper function enables efficient circuit output testing.The
getCircuitOutputValuefunction provides a lightweight way to verify circuit execution and outputs without the overhead of proof generation, which is beneficial for testing validation logic.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 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.
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (5)
examples/CRISP/packages/crisp-sdk/src/vote.ts (5)
decodeTally(97-130)generateMaskVote(221-262)encodeVote(64-90)encryptVoteAndGenerateCRISPInputs(170-208)getCircuitOutputValue(286-292)examples/CRISP/packages/crisp-sdk/src/constants.ts (2)
DEFAULT_BFV_PARAMS(29-29)MESSAGE(34-34)examples/CRISP/packages/crisp-sdk/tests/constants.ts (6)
merkleProof(33-33)testAddress(32-32)VOTE(14-14)votingPowerLeaf(31-31)MESSAGE(11-11)LEAVES(16-27)examples/CRISP/packages/crisp-sdk/tests/utils.ts (1)
compareCoefficientsArrays(14-35)examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(18-20)generateMerkleProof(39-79)
🪛 Gitleaks (8.28.0)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts
[high] 356-356: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 387-387: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 418-418: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (8)
- GitHub Check: build_sdk
- GitHub Check: build_e3_support_dev
- GitHub Check: rust_integration
- GitHub Check: rust_unit
- GitHub Check: build_enclave_cli
- GitHub Check: test_net
- GitHub Check: integration_prebuild
- GitHub Check: test_contracts
🔇 Additional comments (4)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (4)
17-17: LGTM!The import of
getCircuitOutputValueis properly added and used in the new test cases below.
58-120: LGTM!The expanded tally test data and updated expected results (yes: 22n, no: 1n) properly validate the coefficient reversal fix mentioned in the PR objectives.
233-240: LGTM!Formatting improvement for better readability.
330-331: LGTM!Proper boolean comparison syntax improves code clarity.
Also applies to: 348-350
2 participants
fix #974
Summary by CodeRabbit
New Features
Bug Fixes
Tests