test: add test for on-chain zk verification#984
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughReparameterizes verifier constants (N, LOG_N), adds Changes
Sequence Diagram(s)sequenceDiagram
participant User as Client/Test
participant Worker as crisp_worker
participant SDK as crisp-sdk
participant Backend as UltraHonkBackend
participant Verifier as HonkVerifier (contract)
Note over User,Worker: prepare vote, sign message, build Merkle proof
User->>Worker: encrypt request (voteId, publicKey, address, signature, message)
Worker->>SDK: encodeVote(...) and encryptVote(encodedVote, publicKey)
SDK-->>Worker: ciphertext
Worker->>SDK: generateProofWithReturnValue(inputs)
SDK->>Backend: generateProof(witness, { keccak: true })
Backend-->>SDK: proof (includes pairingPointObject[16])
SDK-->>Worker: proofData + returnValue
Worker-->>User: encryptedVote + proofData
User->>Verifier: submit proof + publicInputs (pairing points present)
Verifier-->>User: verification result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
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
feat/onchain-zk-verify
branch
from
5d7cb5c to
5618080
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol(2 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(1 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (4)
examples/CRISP/packages/crisp-sdk/src/vote.ts (3)
encodeVote(64-90)encryptVoteAndGenerateCRISPInputs(170-208)generateProof(264-272)examples/CRISP/packages/crisp-sdk/src/index.ts (1)
VotingMode(14-14)examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
MESSAGE(34-34)examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(18-20)generateMerkleProof(39-79)
🔇 Additional comments (7)
examples/CRISP/packages/crisp-sdk/src/vote.ts (2)
269-269: LGTM: Keccak mode enabled for on-chain verification.The addition of
{ keccak: true }enables Keccak-based proof generation, which is required for on-chain Solidity verification with HonkVerifier.
281-281: LGTM: Consistent update for on-chain verification.The same Keccak mode option is applied here, maintaining consistency with the
generateProoffunction.examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (3)
9-28: LGTM: Test infrastructure properly initialized.The new imports and module-level test fixtures (zkInputsGenerator, publicKey, previousCiphertext) provide the necessary foundation for the proof generation and verification tests.
33-53: LGTM: Deployment test properly validates contract instantiation.The test correctly instantiates CRISPProgram with the required constructor parameters and verifies successful deployment.
103-157: LGTM: Comprehensive on-chain verification test.The test properly exercises the full verification flow from vote creation through on-chain proof verification. The 60-second timeout is appropriate given proof generation complexity.
Note: The PR description mentions this test runs out of gas in some scenarios, which is a known issue being investigated. The test structure itself is sound.
examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol (2)
11-11: LGTM: Verification key updated with production values.The NUMBER_OF_PUBLIC_INPUTS is updated from placeholder value 0 to production value 12304. This large number of public inputs is a contributing factor to the gas costs mentioned in the PR description.
Also applies to: 17-17
18-125: LGTM: Verification key G1Point coordinates updated.The verification key coordinates have been updated from placeholder values to production data generated by the circuit compiler. This data is circuit-specific and enables on-chain verification of CRISP circuit proofs.
cedoor
force-pushed
the
feat/onchain-zk-verify
branch
from
5618080 to
499137c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (1)
102-102: Fix the typo in test description.The test description has a typo: "wi" should be "with".
This issue was previously flagged in an earlier review.
🧹 Nitpick comments (2)
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (1)
23-28: Consider test isolation with global state.The global initialization of
zkInputsGenerator,publicKey, andpreviousCiphertextcreates shared state across tests. If tests run in parallel or in different orders, this could lead to non-deterministic behavior or test interference.Consider moving these initializations into a
beforeEachhook or making them test-local to ensure proper test isolation.examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol (1)
11-11: Public inputs count increased significantly.
NUMBER_OF_PUBLIC_INPUTSchanged from 0 to 12,304, which substantially increases the computational cost of on-chain verification. This aligns with the PR objective but explains the gas limitations mentioned in the PR description.The large number of public inputs will require significant gas for verification, which is a known limitation being addressed in this PR.
Consider exploring batch verification techniques or optimizing the circuit to reduce the number of public inputs if gas costs become prohibitive for production use.
Also applies to: 17-17
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol(2 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(1 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (3)
examples/CRISP/packages/crisp-sdk/src/vote.ts (3)
encodeVote(64-90)encryptVoteAndGenerateCRISPInputs(170-208)generateProof(264-272)examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
MESSAGE(34-34)examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(18-20)generateMerkleProof(39-79)
⏰ 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: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: build_sdk
- GitHub Check: test_net
- GitHub Check: test_contracts
- GitHub Check: rust_unit
- GitHub Check: rust_integration
🔇 Additional comments (7)
examples/CRISP/packages/crisp-sdk/src/vote.ts (2)
264-272: LGTM! Keccak option enabled for proof generation.The addition of
{ keccak: true }enables Keccak-based hashing during proof generation, which is necessary for on-chain verification compatibility.
274-284: LGTM! Consistent Keccak option applied.The Keccak option is consistently applied to both proof generation functions, ensuring uniform behavior across the codebase.
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (3)
33-53: LGTM! Deployment test properly validates contract instantiation.The deployment test now performs actual contract deployment with appropriate constructor parameters and validates the deployment succeeded.
55-99: LGTM! Comprehensive tally decoding validation.The test now includes proper contract deployment and validates tally decoding with multiple concrete test cases, ensuring the decode logic works correctly for different scenarios.
101-157: Test validates end-to-end proof flow but may exceed gas limits.The test successfully validates the complete cryptographic flow from vote encoding through on-chain proof verification. However, per the PR description, this test runs out of gas on-chain due to the complexity of verifying proofs with 12,304 public inputs.
The 60-second timeout accounts for proof generation time but doesn't address the on-chain gas limitation. Consider documenting this known limitation in the test or skipping it in standard test runs until gas optimizations are implemented.
Does this test pass locally with increased gas limits (e.g., using anvil with custom block gas limit)?
examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol (2)
18-129: Verification key updated for new circuit.The complete verification key has been regenerated to match the updated CRISP circuit with 12,304 public inputs. The update appears comprehensive and consistent, with the exception of the qm point noted separately.
34-37: Verify qm in the verifier key (CRISPVerifier.sol).CRISPVerifier.sol contains qm with y = 0; I couldn't find compiled VKs at examples/CRISP/packages/crisp-circuits/target/ (but did find examples/CRISP/client/libs/noir/crisp_circuit.json and examples/CRISP/packages/crisp-contracts/deploy/verify.ts). Confirm that CRISPVerifier.sol was generated from the correct verification key and that qm.y == 0 is intentional (e.g., unused selector or canonical infinity); if not, regenerate the verification key and update the contract.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
examples/CRISP/circuits/.gitignore(1 hunks)examples/CRISP/package.json(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol(13 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(1 hunks)examples/CRISP/packages/crisp-sdk/package.json(1 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/CRISP/packages/crisp-sdk/src/vote.ts
- examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-05T14:12:57.814Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 963
File: examples/CRISP/client/package.json:25-25
Timestamp: 2025-11-05T14:12:57.814Z
Learning: In the Enclave/CRISP codebase, `enclave-e3/sdk` and `crisp-e3/sdk` are different packages: `enclave-e3/sdk` is the general Enclave SDK, while `crisp-e3/sdk` is the CRISP-specific SDK. The CRISP client (`examples/CRISP/client`) intentionally depends on `enclave-e3/sdk`, not `crisp-e3/sdk`.
Applied to files:
examples/CRISP/packages/crisp-sdk/package.json
📚 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/.gitignore
⏰ 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: integration_prebuild
- GitHub Check: test_contracts
- GitHub Check: rust_unit
- GitHub Check: rust_integration
- GitHub Check: test_net
- GitHub Check: build_enclave_cli
- GitHub Check: build_e3_support_risc0
🔇 Additional comments (10)
examples/CRISP/circuits/.gitignore (1)
3-3: LGTM: Appropriate gitignore entry for generated cryptographic parameters.The
crs(Common Reference String) entry correctly ignores generated zero-knowledge proof parameters that should not be version-controlled.examples/CRISP/packages/crisp-sdk/package.json (1)
49-50: Verify compatibility between updated zk library versions.The updates include:
@aztec/bb.js: ^0.82.2 → 0.87.0 (now exact version)@noir-lang/noir_js: 1.0.0-beta.3 → 1.0.0-beta.9Since both are in beta/pre-release stages, ensure these versions are compatible with each other and with the new verification key structure (12321 public inputs). The removal of the caret (^) for bb.js pins it to an exact version, which improves reproducibility.
examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol (7)
566-566: LGTM: Proof size correctly accounts for pairing point object.The increase from 440 to 456 field elements (+16) correctly matches the
PAIRING_POINTS_SIZEconstant, accommodating the newpairingPointObject[16]in the proof structure.
393-394: LGTM: Pairing point object correctly integrated into proof structure.The
pairingPointObjectfield is appropriately typed asFr[PAIRING_POINTS_SIZE]and positioned at the start of the struct, which aligns with the proof loading order implemented at lines 687-690.
1579-1581: LGTM: Public input validation correctly accounts for pairing points.The validation
publicInputs.length != vk.publicInputsSize - PAIRING_POINTS_SIZEcorrectly expects 12,305 public inputs (12,321 - 16), since the pairing point object is embedded in the proof rather than passed as separate public inputs.
683-709: Proof loading reordered to accommodate pairing points, but design may not be final.The proof loading has been restructured to:
- Read 16 pairing point Fr values first (lines 687-690)
- Then read commitments (lines 692-709)
This matches the proof structure, and the boundary tracking appears correct. However, the TODO at line 683 indicates this may be optimized away, suggesting the design is not finalized. Ensure this loading order is tested thoroughly, especially the boundary calculations.
1603-1638: Public input delta calculation correctly handles pairing points, but gas cost is prohibitive.The logic correctly processes:
- Regular public inputs (lines 1615-1623): 12,305 iterations
- Pairing point object (lines 1625-1633): 16 iterations
The field operations in each loop iteration are correctly implemented, but as noted earlier, the 12,305 iterations in the first loop make this function computationally infeasible on-chain. This is the primary cause of the gas exhaustion mentioned in the PR description.
17-124: Verification key updated with new cryptographic commitments.All G1 points in the verification key have been regenerated, which is expected given the circuit now handles 12,321 public inputs instead of 0. These values are circuit-specific cryptographic commitments that must be generated by a trusted circuit compiler. Ensure these values were generated from the correct circuit compilation and that the compilation process is documented and reproducible.
488-498: Array bounds are correct; TODO remains unresolved.Verification confirms the transcript construction logic is sound:
- Array allocation (
3 + publicInputsSize + 12= 12,336 bytes32 elements) matches all writes: 3 prefix + 12,321 public inputs + 12 commitment coordinates- Loop bounds correctly separate regular inputs (indices 3 through 12,307) from pairing points (indices 12,308 through 12,323)
- Design is consistent across validation logic (lines 1579, 1615)
However, the TODO at line 492 regarding whether
publicInputsSizeshould exclude pairing points remains unresolved. This reflects a deferred design question but does not cause functional issues in the current implementation.examples/CRISP/package.json (1)
36-36: ethers ^6.15.0 is the latest stable version with no known security advisories.The dependency is current and secure as of November 2025.
cedoor
force-pushed
the
feat/onchain-zk-verify
branch
from
94e03ea to
4665130
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (1)
102-102: Fix the typo in the test description.The test description contains a typo: "wi" should be "with".
Apply this diff:
- it("should verify the proof correctly with the crisp verifier", async function () { + it("should verify the proof correctly with the CRISP verifier", async function () {
🧹 Nitpick comments (1)
examples/CRISP/packages/crisp-sdk/tests/constants.ts (1)
33-33: LGTM! Merkle proof generation updated correctly.The call now omits the
maxDepthparameter, relying on the internally definedMERKLE_TREE_MAX_DEPTHconstant. This simplifies the API.The
MAX_DEPTHconstant exported at line 29 may no longer be necessary if it's not used elsewhere. Consider removing it to reduce maintenance burden.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
examples/CRISP/circuits/.gitignore(1 hunks)examples/CRISP/circuits/src/main.nr(4 hunks)examples/CRISP/client/libs/wasm/pkg/crisp_worker.js(1 hunks)examples/CRISP/client/package.json(1 hunks)examples/CRISP/client/src/context/voteManagement/VoteManagement.types.ts(1 hunks)examples/CRISP/client/src/hooks/voting/useVoteCasting.ts(1 hunks)examples/CRISP/client/src/hooks/wasm/useWebAssembly.tsx(1 hunks)examples/CRISP/client/src/model/vote.model.ts(1 hunks)examples/CRISP/client/vite.config.ts(1 hunks)examples/CRISP/package.json(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol(13 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(1 hunks)examples/CRISP/packages/crisp-sdk/package.json(1 hunks)examples/CRISP/packages/crisp-sdk/src/constants.ts(1 hunks)examples/CRISP/packages/crisp-sdk/src/utils.ts(3 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(4 hunks)examples/CRISP/packages/crisp-sdk/tests/constants.ts(1 hunks)examples/CRISP/packages/crisp-sdk/tests/utils.test.ts(3 hunks)examples/CRISP/packages/crisp-sdk/tests/vote.test.ts(5 hunks)examples/CRISP/scripts/setup.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/CRISP/packages/crisp-sdk/package.json
- examples/CRISP/package.json
- examples/CRISP/circuits/.gitignore
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-05T14:12:57.814Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 963
File: examples/CRISP/client/package.json:25-25
Timestamp: 2025-11-05T14:12:57.814Z
Learning: In the Enclave/CRISP codebase, `enclave-e3/sdk` and `crisp-e3/sdk` are different packages: `enclave-e3/sdk` is the general Enclave SDK, while `crisp-e3/sdk` is the CRISP-specific SDK. The CRISP client (`examples/CRISP/client`) intentionally depends on `enclave-e3/sdk`, not `crisp-e3/sdk`.
Applied to files:
examples/CRISP/client/package.jsonexamples/CRISP/client/libs/wasm/pkg/crisp_worker.jsexamples/CRISP/client/vite.config.tsexamples/CRISP/scripts/setup.sh
📚 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/client/package.jsonexamples/CRISP/client/vite.config.ts
📚 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:
examples/CRISP/circuits/src/main.nr
🧬 Code graph analysis (10)
examples/CRISP/packages/crisp-sdk/src/vote.ts (2)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (2)
encodedVote(38-38)vote(27-27)examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
DEFAULT_BFV_PARAMS(31-31)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (4)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (3)
merkleProof(32-36)encodedVote(38-38)leaf(30-30)examples/CRISP/packages/crisp-sdk/tests/constants.ts (5)
merkleProof(33-33)votingPowerLeaf(31-31)VOTE(14-14)MESSAGE(11-11)LEAVES(16-27)examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
generateMerkleProof(39-73)hashLeaf(19-21)examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
encodeVote(64-90)
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (4)
examples/CRISP/packages/crisp-sdk/src/vote.ts (3)
encodeVote(64-90)encryptVoteAndGenerateCRISPInputs(182-220)generateProof(276-286)examples/CRISP/packages/crisp-sdk/src/index.ts (1)
VotingMode(14-14)examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
MESSAGE(36-36)examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(19-21)generateMerkleProof(39-73)
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (3)
examples/CRISP/client/src/model/poll.model.ts (1)
Poll(32-36)examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
encryptVote(155-165)examples/CRISP/client/src/model/vote.model.ts (1)
BroadcastVoteRequest(24-30)
examples/CRISP/packages/crisp-sdk/tests/constants.ts (2)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (1)
merkleProof(32-36)examples/CRISP/packages/crisp-sdk/src/utils.ts (1)
generateMerkleProof(39-73)
examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
examples/CRISP/packages/crisp-sdk/src/types.ts (1)
IMerkleProof(59-65)examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
MERKLE_TREE_MAX_DEPTH(13-13)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (2)
examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(19-21)generateMerkleProof(39-73)examples/CRISP/packages/crisp-sdk/src/vote.ts (4)
encodeVote(64-90)encryptVote(155-165)encryptVoteAndGenerateCRISPInputs(182-220)generateProofWithReturnValue(288-300)
examples/CRISP/client/src/hooks/wasm/useWebAssembly.tsx (3)
examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
encryptVote(155-165)examples/CRISP/client/src/model/vote.model.ts (1)
EncryptedVote(56-60)examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (2)
event(19-19)encryptedVote(39-39)
examples/CRISP/packages/crisp-sdk/tests/utils.test.ts (2)
examples/CRISP/packages/crisp-sdk/src/utils.ts (1)
generateMerkleProof(39-73)examples/CRISP/packages/crisp-sdk/tests/constants.ts (1)
LEAVES(16-27)
examples/CRISP/client/src/context/voteManagement/VoteManagement.types.ts (1)
examples/CRISP/client/src/model/vote.model.ts (1)
EncryptedVote(56-60)
🪛 Gitleaks (8.28.0)
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts
[high] 349-349: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 378-378: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 416-416: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 445-445: 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). (9)
- GitHub Check: build_enclave_cli
- GitHub Check: build_sdk
- GitHub Check: build_e3_support_dev
- GitHub Check: integration_prebuild
- GitHub Check: build_e3_support_risc0
- GitHub Check: test_net
- GitHub Check: test_contracts
- GitHub Check: rust_unit
- GitHub Check: rust_integration
🔇 Additional comments (22)
examples/CRISP/scripts/setup.sh (1)
10-11: LGTM! SDK build step properly integrated.The new SDK setup step is correctly positioned after root dependency installation and before the EVM compilation step, ensuring the CRISP SDK is built before dependent components.
examples/CRISP/client/vite.config.ts (1)
26-34: LGTM! Vite optimization excludes updated correctly.The updated exclude list properly prevents pre-bundling of the new CRISP SDK and ZK-related packages, aligning with the dependency changes in this PR. The multi-line format also improves readability.
examples/CRISP/packages/crisp-sdk/tests/vote.test.ts (2)
271-271: LGTM! Merkle proof generation updated correctly.The removal of the
maxDepthparameter aligns with the updatedgenerateMerkleProofsignature that now uses the hardcodedMERKLE_TREE_MAX_DEPTHconstant internally.
349-349: Static analysis false positive - well-known test key.Gitleaks flagged this as a potential API key, but this is Hardhat's well-known first default account private key used universally in Ethereum testing. This is not a security concern.
examples/CRISP/circuits/src/main.nr (1)
121-121: Commented-out return statements require clarification.The return statements are commented out but the conditional logic remains. This suggests the circuit may be transitioning away from explicit returns, possibly using implicit public outputs instead.
Please confirm:
- Are these returns intentionally removed or is this work-in-progress?
- How does the verifier now extract the ciphertext outputs (ct0is, ct1is, sum_ct0is, sum_ct1is)?
- Do the commented returns need to be removed entirely or restored?
Also applies to: 131-131, 135-135
examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
13-14: LGTM! Merkle tree depth properly centralized.Exporting
MERKLE_TREE_MAX_DEPTHas a constant is good practice, ensuring consistency between the SDK and the circuit's hardcoded depth of 20.examples/CRISP/client/src/model/vote.model.ts (1)
25-29: LGTM! Stylistic cleanup.Removing trailing semicolons from interface properties is a purely stylistic change with no functional impact.
Also applies to: 32-32, 34-36
examples/CRISP/client/package.json (1)
22-22: This dependency change contradicts documented learning and appears unused in the client code.Based on verification:
@crisp-e3/sdkis listed inpackage.jsonbut is not imported or used anywhere in the client source code- The client uses a local WebAssembly worker and HTTP calls to the Enclave API, not the SDK directly
- Recent learning from this codebase (Nov 5, 2025) explicitly states: "CRISP client intentionally depends on
@enclave-e3/sdk, not@crisp-e3/sdk"- This change replaces
@enclave-e3/sdkwith@crisp-e3/sdkEither the learning is outdated, or this dependency addition is incomplete/incorrect. Verify that:
- This change was intentional (replacing the entire SDK dependency)
- The client code will work with
@crisp-e3/sdkas intended- Build and runtime tests pass
examples/CRISP/packages/crisp-sdk/src/utils.ts (1)
57-60: LGTM! Padding logic correctly uses the constant.The padding calculations for siblings and indices properly use
MERKLE_TREE_MAX_DEPTHto ensure fixed-size arrays for the circuit.examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (4)
9-28: LGTM! Test setup correctly initializes ZK inputs generator.The initialization of
zkInputsGenerator,publicKey, andpreviousCiphertextat the module level is appropriate for reuse across tests. The encrypted zero vector serves as a proper initial state for the first vote.
33-53: LGTM! Deployment test correctly validates contract initialization.The test properly deploys the CRISPProgram contract with mock dependencies and verifies the deployment succeeded.
55-98: LGTM! Tally decoding test correctly verifies binary-to-decimal conversion.The test validates the tally decoding logic with two different examples, properly checking both yes and no vote counts.
136-156: LGTM! Proof generation and verification flow is correct.The test properly constructs CRISP inputs, generates a ZK proof, and verifies it on-chain using the HonkVerifier. The 60-second timeout is appropriate for proof generation.
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (4)
7-22: LGTM! Hook dependencies and imports are correctly configured.The hook properly imports and initializes all required dependencies including the new
useSignMessagehook for signature generation.
24-50: LGTM! Input validation and message signing are correctly implemented.The validation checks for poll selection and user state are comprehensive with clear error messages. The message signing flow properly constructs a round-specific message.
52-108: LGTM! Error handling is comprehensive and properly manages loading state.The try/catch/finally structure ensures
isLoadingis always reset even if errors occur, preventing stuck loading states. The broadcast response handling covers multiple cases (success, user_already_voted, failed_broadcast, default error) with appropriate user feedback.
110-111: LGTM! Dependency array is complete.All external values used in the callback are properly included in the dependency array, ensuring the callback updates when dependencies change.
examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol (5)
8-10: LGTM! Constants are consistent and address previous gas concerns.The reduction of
NUMBER_OF_PUBLIC_INPUTSfrom 12,321 to 16 directly addresses the critical gas concern raised in the previous review. The new constantPAIRING_POINTS_SIZE = 16matchesNUMBER_OF_PUBLIC_INPUTS, and circuit parameters are internally consistent (N = 2^19).Also applies to: 14-16, 275-275
392-414: LGTM! Proof struct correctly adds pairing point object.The
pairingPointObjectfield is appropriately typed asFr[PAIRING_POINTS_SIZE]and positioned as the first field in the struct, which aligns with the updated proof loading logic.
682-709: LGTM! Proof loading correctly handles pairing point object.The
PROOF_SIZEincrease from 440 to 456 (+16 field elements) correctly accounts for the newpairingPointObject. TheloadProoffunction properly reads the pairing point fields first before the commitments, maintaining consistent boundary tracking.Also applies to: 1566-1566
488-498: LGTM! Transcript generation correctly separates pairing points.The eta challenge generation properly processes regular public inputs separately from pairing point fields. The TODO comment at line 492 indicates awareness that the pairing point size handling might be refined in the future.
1579-1591: LGTM! Public input delta computation correctly handles pairing points.The refactored
computePublicInputDeltafunction properly processes two input categories:
- Regular public inputs from the
publicInputsarray (lines 1615-1623)- Pairing point fields from
proof.pairingPointObject(lines 1625-1633)The length validation at line 1579 correctly checks only the regular public inputs against
vk.publicInputsSize - PAIRING_POINTS_SIZE. Both loops maintain the same accumulation pattern, ensuring mathematical correctness.Also applies to: 1603-1638
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (1)
23-28: Consider moving initialization to beforeEach for better test isolation.The top-level initialization of
zkInputsGenerator,publicKey, andpreviousCiphertextcreates shared state across all tests. If tests run in parallel or modify these values, it could lead to flaky tests or unexpected interactions.Consider moving this initialization into a
beforeEachhook:+describe("CRISP Contracts", function () { + let zkInputsGenerator: ZKInputsGenerator; + let publicKey: any; + let previousCiphertext: any; + + beforeEach(() => { + zkInputsGenerator = ZKInputsGenerator.withDefaults(); + publicKey = zkInputsGenerator.generatePublicKey(); + previousCiphertext = zkInputsGenerator.encryptVote( + publicKey, + new BigInt64Array([0n]) + ); + }); + -let zkInputsGenerator = ZKInputsGenerator.withDefaults(); -let publicKey = zkInputsGenerator.generatePublicKey(); -const previousCiphertext = zkInputsGenerator.encryptVote( - publicKey, - new BigInt64Array([0n]) -); - -describe("CRISP Contracts", function () {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js(1 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-05T14:12:57.814Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 963
File: examples/CRISP/client/package.json:25-25
Timestamp: 2025-11-05T14:12:57.814Z
Learning: In the Enclave/CRISP codebase, `enclave-e3/sdk` and `crisp-e3/sdk` are different packages: `enclave-e3/sdk` is the general Enclave SDK, while `crisp-e3/sdk` is the CRISP-specific SDK. The CRISP client (`examples/CRISP/client`) intentionally depends on `enclave-e3/sdk`, not `crisp-e3/sdk`.
Applied to files:
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js
🧬 Code graph analysis (2)
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (4)
examples/CRISP/packages/crisp-sdk/src/vote.ts (3)
encodeVote(64-90)encryptVoteAndGenerateCRISPInputs(182-220)generateProof(276-286)examples/CRISP/packages/crisp-sdk/src/index.ts (1)
VotingMode(14-14)examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
MESSAGE(36-36)examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(19-21)generateMerkleProof(39-73)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (2)
examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(19-21)generateMerkleProof(39-73)examples/CRISP/packages/crisp-sdk/src/vote.ts (4)
encodeVote(64-90)encryptVote(155-165)encryptVoteAndGenerateCRISPInputs(182-220)generateProofWithReturnValue(288-300)
⏰ 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_enclave_cli
- GitHub Check: integration_prebuild
- GitHub Check: test_net
- GitHub Check: build_sdk
- GitHub Check: rust_unit
- GitHub Check: rust_integration
- GitHub Check: test_contracts
🔇 Additional comments (3)
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (2)
123-125: Verify leaves construction for semantic correctness.The leaves array construction mixes a properly hashed leaf with raw bigint values (10n, 20n):
const leaf = hashLeaf(address, vote.yes.toString()); const leaves = [...[10n, 20n], leaf];The
hashLeaffunction produces a large hash value, but10nand20nare raw bigints. For semantic correctness, if these values represent other users in the merkle tree, they should also be hashed usinghashLeaf(address, balance).The test may still pass since
generateMerkleProofwill find the hashed leaf at index 2 and build a valid proof, but this construction is semantically inconsistent with a real merkle tree of user balances.If 10n and 20n are intentional dummy values for testing, consider adding a comment to clarify. Otherwise, verify whether all leaves should be properly hashed:
const leaves = [ hashLeaf(otherAddress1, "10"), hashLeaf(otherAddress2, "20"), leaf ];
128-156: LGTM! Past function signature issue resolved, proof verification flow is correct.The test correctly:
- Calls
generateMerkleProofwith the updated 4-parameter signature (past 5-parameter issue has been resolved)- Generates CRISP inputs with all required parameters
- Generates and verifies the zk proof via HonkVerifier
- Sets an appropriate 60-second timeout for proof generation
The proof verification flow is well-structured and properly tests the on-chain zk verification functionality.
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (1)
30-34: Address normalization fix looks solid.Lowercasing before hashing and Merkle proof generation keeps the leaf consistent with
generateMerkleProof’s internal hashing, eliminating the prior “leaf not found” failure for mixed-case addresses.
- Updated @crisp-e3/sdk to 0.1.0-test - Updated @crisp-e3/contracts to 0.1.0-test - Updated @crisp-e3/zk-inputs to 0.1.0-test - Published to npm
- Updated @crisp-e3/sdk to 0.2.0-test - Updated @crisp-e3/contracts to 0.2.0-test - Updated @crisp-e3/zk-inputs to 0.2.0-test - Published to npm
cedoor
force-pushed
the
feat/onchain-zk-verify
branch
from
80bb577 to
992dc4c
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
examples/CRISP/circuits/src/main.nr (1)
21-22: Reminder: Circuit interface changes pending future PR.As noted in the previous review, the removal of
pubvisibility from parameters and commented-out return statements break the circuit's public interface and cause test failures. The maintainer has acknowledged this and plans to address it in upcoming PRs.For tracking purposes, the following items remain to be addressed:
- Restore
pubvisibility on parameters that must be public inputs- Uncomment and implement return statements (lines 121, 131, 135)
- Verify verifier contract compatibility with final circuit interface
- Ensure
generateProofWithReturnValue()tests passAlso applies to: 28-30, 48-48, 53-53, 57-58, 121-121, 131-131, 135-135
🧹 Nitpick comments (3)
examples/CRISP/test/crisp.spec.ts (1)
130-132: Consider documenting the 5-minute wait time.The 5-minute wait after signature confirmation is likely necessary for ZK proof generation and on-chain verification, but a comment explaining this would improve test maintainability.
Consider adding a comment:
+ // Wait for ZK proof generation, on-chain submission, and verification const WAIT = 300_000; log(`waiting for ${WAIT}ms...`); await page.waitForTimeout(WAIT);Alternatively, consider making this configurable via an environment variable for different test environments.
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (1)
23-28: Consider moving shared state into test scope for better isolation.The
zkInputsGenerator,publicKey, andpreviousCiphertextare initialized at module scope and shared across tests. IfZKInputsGeneratormaintains internal state, this could lead to test interdependencies.Consider moving these initializations into a
beforeEachhook or directly into the test that uses them:-let zkInputsGenerator = ZKInputsGenerator.withDefaults(); -let publicKey = zkInputsGenerator.generatePublicKey(); -const previousCiphertext = zkInputsGenerator.encryptVote( - publicKey, - new BigInt64Array([0n]) -); - describe("CRISP Contracts", function () { const nonZeroAddress = "0xc6e7DF5E7b4f2A278906862b61205850344D4e7d"; + + let zkInputsGenerator: ZKInputsGenerator; + let publicKey: any; + let previousCiphertext: any; + + beforeEach(() => { + zkInputsGenerator = ZKInputsGenerator.withDefaults(); + publicKey = zkInputsGenerator.generatePublicKey(); + previousCiphertext = zkInputsGenerator.encryptVote( + publicKey, + new BigInt64Array([0n]) + ); + });examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (1)
34-50: Validation flow looks good, minor comment wording issue.The validation checks for poll selection, user, and round state are comprehensive and provide appropriate user feedback. The message signing integration is correct.
Minor: The comment on line 48 has a double negative ("do not do nothing"). Consider:
- // For now just sign and do not do nothing with the signature + // Sign the message (signature will be used in vote encryption and broadcast)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
examples/CRISP/circuits/.gitignore(1 hunks)examples/CRISP/circuits/src/main.nr(4 hunks)examples/CRISP/client/libs/wasm/pkg/crisp_worker.js(1 hunks)examples/CRISP/client/package.json(1 hunks)examples/CRISP/client/src/context/voteManagement/VoteManagement.types.ts(1 hunks)examples/CRISP/client/src/hooks/voting/useVoteCasting.ts(1 hunks)examples/CRISP/client/src/hooks/wasm/useWebAssembly.tsx(1 hunks)examples/CRISP/client/src/model/vote.model.ts(1 hunks)examples/CRISP/client/vite.config.ts(2 hunks)examples/CRISP/package.json(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol(13 hunks)examples/CRISP/packages/crisp-contracts/package.json(1 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(1 hunks)examples/CRISP/packages/crisp-sdk/package.json(3 hunks)examples/CRISP/packages/crisp-sdk/src/constants.ts(1 hunks)examples/CRISP/packages/crisp-sdk/src/utils.ts(3 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(4 hunks)examples/CRISP/packages/crisp-sdk/tests/constants.ts(1 hunks)examples/CRISP/packages/crisp-sdk/tests/utils.test.ts(3 hunks)examples/CRISP/packages/crisp-sdk/tests/vote.test.ts(5 hunks)examples/CRISP/packages/crisp-zk-inputs/package.json(1 hunks)examples/CRISP/scripts/setup.sh(1 hunks)examples/CRISP/test/crisp.spec.ts(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/CRISP/client/src/model/vote.model.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- examples/CRISP/packages/crisp-sdk/tests/vote.test.ts
- examples/CRISP/client/vite.config.ts
- examples/CRISP/packages/crisp-sdk/tests/utils.test.ts
- examples/CRISP/packages/crisp-sdk/src/constants.ts
- examples/CRISP/packages/crisp-contracts/package.json
- examples/CRISP/package.json
- examples/CRISP/packages/crisp-sdk/tests/constants.ts
- examples/CRISP/packages/crisp-sdk/package.json
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-08T01:31:47.484Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 969
File: packages/enclave-sdk/src/enclave-sdk.ts:101-103
Timestamp: 2025-11-08T01:31:47.484Z
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:
examples/CRISP/packages/crisp-sdk/src/vote.ts
📚 Learning: 2025-11-05T14:12:57.814Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 963
File: examples/CRISP/client/package.json:25-25
Timestamp: 2025-11-05T14:12:57.814Z
Learning: In the Enclave/CRISP codebase, `enclave-e3/sdk` and `crisp-e3/sdk` are different packages: `enclave-e3/sdk` is the general Enclave SDK, while `crisp-e3/sdk` is the CRISP-specific SDK. The CRISP client (`examples/CRISP/client`) intentionally depends on `enclave-e3/sdk`, not `crisp-e3/sdk`.
Applied to files:
examples/CRISP/client/libs/wasm/pkg/crisp_worker.jsexamples/CRISP/client/package.jsonexamples/CRISP/scripts/setup.sh
📚 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:
examples/CRISP/circuits/src/main.nr
📚 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/.gitignore
🧬 Code graph analysis (7)
examples/CRISP/client/src/context/voteManagement/VoteManagement.types.ts (1)
examples/CRISP/client/src/model/vote.model.ts (1)
EncryptedVote(56-60)
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (3)
examples/CRISP/client/src/model/poll.model.ts (1)
Poll(32-36)examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
encryptVote(155-165)examples/CRISP/client/src/model/vote.model.ts (1)
BroadcastVoteRequest(24-30)
examples/CRISP/packages/crisp-sdk/src/vote.ts (2)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (2)
encodedVote(38-38)vote(27-27)examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
DEFAULT_BFV_PARAMS(31-31)
examples/CRISP/client/src/hooks/wasm/useWebAssembly.tsx (3)
examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
encryptVote(155-165)examples/CRISP/client/src/model/vote.model.ts (1)
EncryptedVote(56-60)examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (2)
event(19-19)encryptedVote(39-39)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (2)
examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(19-21)generateMerkleProof(39-73)examples/CRISP/packages/crisp-sdk/src/vote.ts (4)
encodeVote(64-90)encryptVote(155-165)encryptVoteAndGenerateCRISPInputs(182-220)generateProofWithReturnValue(288-300)
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (3)
examples/CRISP/packages/crisp-sdk/src/vote.ts (3)
encodeVote(64-90)encryptVoteAndGenerateCRISPInputs(182-220)generateProof(276-286)examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
MESSAGE(36-36)examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
hashLeaf(19-21)generateMerkleProof(39-73)
examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
examples/CRISP/packages/crisp-sdk/src/types.ts (1)
IMerkleProof(59-65)examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
MERKLE_TREE_MAX_DEPTH(13-13)
⏰ 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: crisp_e2e
- GitHub Check: rust_integration
- GitHub Check: test_net
🔇 Additional comments (15)
examples/CRISP/packages/crisp-zk-inputs/package.json (1)
5-5: Version bump coordinated with broader CRISP ecosystem update.The version update from 0.0.2-test to 0.2.0-test aligns with the coordinated version bumps across CRISP packages mentioned in the PR. This appears appropriate given the significant feature additions throughout the ecosystem.
Please verify that all CRISP packages (crisp-sdk, crisp-contracts, and crisp-zk-inputs) maintain consistent version numbering (0.2.0-test) across the PR.
examples/CRISP/circuits/.gitignore (1)
1-3: LGTM!The addition of
crsto the ignore list is appropriate for ZK proof projects where Common Reference String files are typically generated at runtime and should not be committed.examples/CRISP/test/crisp.spec.ts (1)
128-129: LGTM! Signature confirmation correctly added.The addition of the MetaMask signature confirmation step aligns with the PR's objective to test on-chain ZK verification. The logging and async handling are appropriate.
examples/CRISP/scripts/setup.sh (1)
10-11: Style and error handling are consistent with the script pattern.The new SDK setup phase follows the existing conventions: clear labeling with
echo, error handling viaset -e, and subshell isolation. The command chaining (pnpm install && pnpm build) is appropriate for dependency resolution before build.examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (1)
101-156: Well-structured end-to-end verification test.The test properly covers the complete flow: vote encoding → signature generation → Merkle proof → CRISP input generation → proof generation → on-chain verification. The 60-second timeout is appropriate for proof generation.
examples/CRISP/client/src/hooks/wasm/useWebAssembly.tsx (3)
27-33: LGTM! Signature correctly extended with authentication parameters.The addition of
address,signature, andmessageparameters aligns with the broader PR changes to thread authentication data through the vote encryption flow.
41-41: LGTM! Worker message payload correctly updated.The postMessage call properly includes the new
address,signature, andmessagefields in the data payload.
46-48: LGTM! Destructuring cleanup looks good.The destructuring pattern correctly extracts
voteandproofData, then further destructuresproofandpublicInputsfrom the nested object.examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (4)
7-14: LGTM! Imports correctly updated for new signing flow.The addition of
useSignMessagefrom wagmi and other hooks properly supports the expanded vote casting functionality.
24-30: LGTM! Well-structured encryption helper.The
handleVoteEncryptioncallback properly encapsulates the encryption logic with appropriate validation and error handling.
52-108: LGTM! Robust error handling and response management.The encryption, broadcast, and response handling flow is well-structured:
- Proper error propagation with early throws
- Comprehensive switch statement covering success, user_already_voted, failed_broadcast, and default cases
- Appropriate user feedback via toasts
- Loading state correctly reset in finally block
- Navigation only on successful vote submission
110-110: LGTM! Dependency array is complete and correct.All values used within the
castVoteWithProofcallback are properly included in the dependency array.examples/CRISP/packages/crisp-sdk/src/utils.ts (3)
11-11: LGTM! Constant import aligns with the refactor.The import of
MERKLE_TREE_MAX_DEPTHfrom constants properly supports the removal of themaxDepthparameter.
39-39: LGTM! Signature correctly refactored to use constant.The removal of the
maxDepthparameter in favor ofMERKLE_TREE_MAX_DEPTHimproves consistency across the codebase. The past review concern about call site updates was addressed in commit 62ae49b.
57-60: LGTM! Constant correctly applied in padding logic.Both
paddedSiblingsandpaddedIndicescalculations properly useMERKLE_TREE_MAX_DEPTHfor consistent depth handling.
3 participants
Closes #958
Summary by CodeRabbit
New Features
Tests
Chores