refactor: bind encrypted vote to zk proof [skip-line-limit]#1025
Conversation
WalkthroughAdds a public Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Worker as CRISP Worker
participant SDK
participant Noir as Noir Circuit
participant Program as CRISP Program / Contract
Client->>Worker: postMessage { type: "generate_proof", inputs }
Worker->>SDK: generateProof(inputs)
SDK->>Noir: run circuit -> ProofData (proof + publicInputs)
Noir-->>SDK: ProofData
SDK->>SDK: encodeSolidityProof(ProofData) → encodedProof (hex)
SDK-->>Worker: encodedProof
Worker-->>Client: postMessage { type: "generate_proof", result: encodedProof }
Client->>Program: publishInput(encodedProof)
Program->>Program: abi.decode(bytes, bytes32[], address) -> (proof, vote, slotAddress)
Program->>Program: abi_decode_greco_ciphertext(proof) -> ct0is, ct1is
Program->>Program: greco_to_bfv_ciphertext(ct0is, ct1is, params) -> BFV Ciphertext
Program->>Program: process/verify using slotAddress and vote
Program-->>Client: success / revert
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related issues
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
cedoor
force-pushed
the
refactor/ct-format
branch
from
93103b9 to
3a1253f
Compare
cedoor
force-pushed
the
refactor/ct-format
branch
from
3a1253f to
541cbeb
Compare
cedoor
force-pushed
the
refactor/ct-format
branch
from
541cbeb to
a1dd55c
Compare
cedoor
force-pushed
the
refactor/ct-format
branch
from
a1dd55c to
2bf2efa
Compare
cedoor
force-pushed
the
refactor/ct-format
branch
from
2bf2efa to
3bdd19f
Compare
cedoor
force-pushed
the
refactor/ct-format
branch
from
3bdd19f to
1f30ba7
Compare
cedoor
changed the title
|
❌ License Header Check Failed Some files are missing the required SPDX license header. Please add the following header to the beginning of all You can run Or run |
- Updated @crisp-e3/sdk to 0.3.0-test - Updated @crisp-e3/contracts to 0.3.0-test - Updated @crisp-e3/zk-inputs to 0.3.0-test - Published to npm
![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
♻️ Duplicate comments (1)
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (1)
45-56: MovesignMessageAsyncinside the try/catch/finally block.The
signMessageAsynccall at lines 49-50 is still outside thetryblock, despite previous review feedback. If the user rejects the signature or the wallet throws an error, execution will exit before reaching thefinallyblock at line 104, leavingisLoadingstuck attrue.Apply this diff to move signature creation inside the try block:
setIsLoading(true) console.log('Processing vote...') - // For now just sign and do not do nothing with the signature - const message = `Vote for round ${roundState.id}` - const signature = await signMessageAsync({ message }) - try { + // For now just sign and do not do nothing with the signature + const message = `Vote for round ${roundState.id}` + const signature = await signMessageAsync({ message }) + const encodedProof = await handleProofGeneration(pollSelected, user.address, signature, message)
🧹 Nitpick comments (3)
examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol (1)
10-129: Public input size & VK constants look internally consistent; keep NUM_PUBLIC_INPUTS and vk.publicInputsSize tightly coupled
NUMBER_OF_PUBLIC_INPUTS(line 10) andvk.publicInputsSize(line 17) are both set to2066, which keeps:
- the public input length check in
verify()(vk.publicInputsSize - PAIRING_POINTS_SIZE→2050),- the loops in
ZKTranscriptLib.generateEtaChallenge(usingpublicInputsSizeandPAIRING_POINTS_SIZE),- and
computePublicInputDelta(which uses$NUM_PUBLIC_INPUTS) all in sync.The new
VK_HASHand all updated G1 constants (ql/qr/qo/q4/qm/qc, lookup/arithmetic/range/elliptic/memory/nnf/Poseidon selectors,s1–s4,id1–id4,lagrangeLast) are wired consistently with the rest of the verifier and will be curve-validated duringbatchMul, so any malformed point will causeShpleminiFailedrather than silent mis-verification.The main fragility now is semantic:
NUMBER_OF_PUBLIC_INPUTSandvk.publicInputsSizemust always be updated together from the same vk-generation pipeline; if one changes without the other,verify()’s input-length check and the transcript/delta code will diverge. As a (non-blocking) follow-up, consider adding a small regression test on the contract side that assertsNUMBER_OF_PUBLIC_INPUTS == HonkVerificationKey.loadVerificationKey().publicInputsSizeto catch accidental desyncs when regenerating the VK.crates/bfv-helpers/Cargo.toml (1)
22-22: Pin the git dependency to a specific commit or tag.The
shareddependency references a git repository without specifying a commit, tag, or branch. This can lead to non-reproducible builds if the remote repository changes.Apply this pattern to pin to a specific revision:
-shared = { package = "zkfhe-shared", git = "https://github.com/gnosisguild/zkfhe-generator" } +shared = { package = "zkfhe-shared", git = "https://github.com/gnosisguild/zkfhe-generator", rev = "COMMIT_HASH" }Or use a tag:
-shared = { package = "zkfhe-shared", git = "https://github.com/gnosisguild/zkfhe-generator" } +shared = { package = "zkfhe-shared", git = "https://github.com/gnosisguild/zkfhe-generator", tag = "v1.0.0" }examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol (1)
142-146: Consider removing unused decode operation.The function decodes the input data but doesn't use the extracted values (
voteis unused). Since the return value was removed, the decode operation only serves to validate the data structure.If the decode is purely for validation, consider adding a comment to clarify:
function validateInput(uint256 e3Id, address, bytes memory data) external { if (data.length == 0) revert EmptyInputData(); + // Decode to validate structure (will revert if malformed) (, , bytes memory vote, ) = abi.decode(data, (bytes, bytes32[], bytes, address)); }Alternatively, if the decode isn't needed for validation, it could be removed to save gas:
function validateInput(uint256 e3Id, address, bytes memory data) external { if (data.length == 0) revert EmptyInputData(); - - (, , bytes memory vote, ) = abi.decode(data, (bytes, bytes32[], bytes, address)); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
Cargo.lockis excluded by!**/*.lockexamples/CRISP/Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yamltemplates/default/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
crates/bfv-helpers/Cargo.toml(1 hunks)crates/bfv-helpers/src/lib.rs(1 hunks)crates/bfv-helpers/src/utils/greco.rs(1 hunks)crates/bfv-helpers/src/utils/mod.rs(2 hunks)docs/pages/write-e3-contract.mdx(1 hunks)examples/CRISP/circuits/src/main.nr(3 hunks)examples/CRISP/client/libs/crispWorker.js(2 hunks)examples/CRISP/client/package.json(1 hunks)examples/CRISP/client/src/context/voteManagement/VoteManagement.context.tsx(2 hunks)examples/CRISP/client/src/context/voteManagement/VoteManagement.types.ts(2 hunks)examples/CRISP/client/src/hooks/voting/useVoteCasting.ts(3 hunks)examples/CRISP/client/src/hooks/wasm/useWebAssembly.tsx(3 hunks)examples/CRISP/client/src/model/vote.model.ts(1 hunks)examples/CRISP/enclave.config.yaml(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol(2 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol(1 hunks)examples/CRISP/packages/crisp-contracts/package.json(1 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(6 hunks)examples/CRISP/packages/crisp-contracts/tests/utils.ts(1 hunks)examples/CRISP/packages/crisp-sdk/package.json(1 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(2 hunks)examples/CRISP/packages/crisp-zk-inputs/package.json(1 hunks)examples/CRISP/program/src/lib.rs(1 hunks)examples/CRISP/server/src/server/models.rs(1 hunks)examples/CRISP/server/src/server/routes/voting.rs(3 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json(1 hunks)packages/enclave-contracts/contracts/interfaces/IE3Program.sol(1 hunks)packages/enclave-contracts/contracts/test/MockE3Program.sol(1 hunks)templates/default/contracts/MyProgram.sol(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json
- examples/CRISP/client/src/model/vote.model.ts
- examples/CRISP/server/src/server/models.rs
- examples/CRISP/client/src/context/voteManagement/VoteManagement.context.tsx
- examples/CRISP/circuits/src/main.nr
- crates/bfv-helpers/src/utils/greco.rs
- crates/bfv-helpers/src/utils/mod.rs
- examples/CRISP/packages/crisp-sdk/package.json
- examples/CRISP/packages/crisp-contracts/tests/utils.ts
- examples/CRISP/enclave.config.yaml
- templates/default/contracts/MyProgram.sol
- packages/enclave-contracts/contracts/interfaces/IE3Program.sol
- examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts
- docs/pages/write-e3-contract.mdx
- examples/CRISP/packages/crisp-contracts/package.json
- examples/CRISP/client/src/context/voteManagement/VoteManagement.types.ts
- examples/CRISP/packages/crisp-zk-inputs/package.json
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
📚 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.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:
crates/bfv-helpers/Cargo.toml
📚 Learning: 2025-11-12T10:08:30.720Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 996
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:144-169
Timestamp: 2025-11-12T10:08:30.720Z
Learning: In the CRISP program (examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol), there are plans to move the merkle tree from the Enclave contract to the program contract itself in the future, which would allow direct calls to validateInput to be stored in the merkle tree.
Applied to files:
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.solpackages/enclave-contracts/contracts/test/MockE3Program.solexamples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol
📚 Learning: 2024-09-26T03:11:29.311Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-09-26T03:11:29.311Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Applied to files:
examples/CRISP/server/src/server/routes/voting.rscrates/bfv-helpers/src/lib.rsexamples/CRISP/program/src/lib.rs
📚 Learning: 2024-10-16T09:51:10.038Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/router/src/committee_meta.rs:43-43
Timestamp: 2024-10-16T09:51:10.038Z
Learning: In `packages/ciphernode/router/src/committee_meta.rs`, avoid suggesting making the `on_event` method in `CommitteMetaFeature` asynchronous or adding error handling for the `write` operation to the data store.
Applied to files:
examples/CRISP/server/src/server/routes/voting.rs
📚 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/program/src/lib.rs
🧬 Code graph analysis (3)
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (4)
examples/CRISP/crates/evm_helpers/src/lib.rs (1)
address(59-61)packages/enclave-sdk/src/index.ts (1)
generateProof(69-69)packages/enclave-sdk/src/greco.ts (1)
generateProof(122-159)examples/CRISP/client/src/model/vote.model.ts (1)
BroadcastVoteRequest(24-28)
examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
examples/CRISP/client/libs/crispWorker.js (2)
proof(53-53)vote(27-27)
examples/CRISP/server/src/server/routes/voting.rs (4)
examples/CRISP/client/src/model/vote.model.ts (1)
VoteResponseStatus(30-30)examples/CRISP/client/libs/crispWorker.js (1)
data(23-23)examples/CRISP/server/src/server/repo.rs (3)
store(35-37)store(78-80)has_voted(228-231)examples/CRISP/server/src/server/models.rs (1)
from(194-204)
⏰ 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). (11)
- GitHub Check: Build & Push Image
- GitHub Check: rust_integration
- GitHub Check: build_e3_support_dev
- GitHub Check: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: build_ciphernode_image
- GitHub Check: test_net
- GitHub Check: build_sdk
- GitHub Check: test_contracts
- GitHub Check: rust_unit
- GitHub Check: crisp_rust_unit
🔇 Additional comments (29)
examples/CRISP/client/package.json (1)
21-21: LGTM: Dependency version update aligns with SDK changes.The version bump from "0.2.3-test" to "0.3.0-test" is consistent with the new
encodeSolidityProoffunctionality added to the SDK.crates/bfv-helpers/Cargo.toml (1)
18-21: LGTM: New dependencies support GRECO/BFV conversion.The added dependencies (itertools, ndarray, num-traits) are well-established crates appropriate for the new greco module's mathematical operations.
crates/bfv-helpers/src/lib.rs (1)
8-8: LGTM: Module exposure enables greco functionality access.Changing from private
mod utilto publicpub mod utilscorrectly exposes the new greco conversion utilities. The plural naming follows Rust conventions.packages/enclave-contracts/contracts/test/MockE3Program.sol (1)
36-44: LGTM: Mock signature updated to match interface.The removal of the return value aligns with the updated
IE3Programinterface. Validation logic remains intact.examples/CRISP/program/src/lib.rs (2)
21-22: LGTM: GRECO-to-BFV conversion properly integrated.The two-step conversion correctly implements the binding of ciphertext to the zk proof as described in the PR objectives. The greco utility functions include unit tests for validation.
As mentioned in the PR objectives, @hmzakhalid should verify the RISC0 cycle count impact of these new functions to ensure the optimization goals are met.
27-27: LGTM: Explicit return improves clarity.The explicit
sum.to_bytes()return statement is more idiomatic Rust than the previous implicit return.examples/CRISP/client/src/hooks/wasm/useWebAssembly.tsx (2)
17-17: LGTM: Worker path updated.The path change from
'libs/wasm/pkg/crisp_worker.js'to'libs/crispWorker.js'aligns with the restructuring shown in the related files.
26-62: LGTM: API refactored to return encoded proof string.The function rename from
encryptVotetogenerateProofand the return type change toPromise<string | undefined>correctly reflect the new workflow where the proof is Solidity-encoded. The implementation consistently updates:
- Message type:
'encrypt_vote'→'generate_proof'- Result destructuring:
vote, proof→encodedProof- Return value: Complex object → string
- Error handling references
examples/CRISP/server/src/server/routes/voting.rs (8)
7-18: LGTM: Imports updated correctly for new encoded_proof flow.The imports have been appropriately updated to use
VoteRequestinstead ofEncryptedVoteand simplified the alloy primitives to justBytesandU256, consistent with the new approach where proof encoding happens on the client side.
26-43: LGTM: Function signature and logging updated correctly.The function signature has been properly updated to use
VoteRequest, and all logging statements correctly referencevote_request.round_id.
44-67: LGTM: Vote status validation is correct.The duplicate vote check properly uses
vote_request.round_idandvote_request.addresswith appropriate error handling and early return.
69-80: LGTM: Voter insertion is correctly implemented.The voter address is inserted into the database with proper error handling. This insertion point is important for rollback verification in error paths.
82-107: LGTM: Hex decoding with proper error handling and rollback.The hex decoding properly handles errors and performs rollback of the voter insertion before returning a
BadRequestresponse. This addresses the critical issue from previous reviews.
109-130: LGTM: Contract creation with proper error handling and rollback.Contract creation error handling correctly performs rollback of the voter insertion before returning an
InternalServerError. The error message accurately reflects the failure.
132-146: LGTM: Vote broadcast correctly integrated with error handling.The broadcast operation uses the decoded
encoded_proofand properly delegates error handling tohandle_vote_error, which will rollback the voter insertion.
148-174: LGTM: Well-designed error handler with rollback.The
handle_vote_errorhelper properly encapsulates the rollback logic and provides a controlled error response. This promotes consistency across error paths.examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (3)
17-17: LGTM: Context updated to use generateProof.The hook correctly destructures
generateProoffrom the vote management context, aligning with the PR's shift from encryption to proof generation.
24-30: LGTM: Proof generation handler correctly implemented.The
handleProofGenerationfunction properly wraps thegenerateProofcall with appropriate error handling and updated error messages.
58-109: LGTM: Vote request payload and response handling updated correctly.The
BroadcastVoteRequestpayload correctly usesencoded_proofinstead of separateenc_vote_bytesandprooffields. The broadcast response handling and useCallback dependencies are properly updated.examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol (5)
145-145: LGTM: Signature change aligns with PR objectives.The
validateInputfunction no longer returns the input bytes, which is correct since the ciphertext is now bound directly to the proof and emitted in the event instead.
151-158: LGTM: Input decoding correctly extracts proof-bound ciphertext.The input data is properly decoded to extract the Noir proof, the vote (as
bytes32[]), and the slot address. Re-encoding the vote to bytes for storage and event emission is necessary since the individual elements are used for proof verification.
159-169: LGTM: Public inputs correctly bind ciphertext to proof.The Noir public inputs construction properly includes the vote ciphertext elements (starting at index 2), ensuring the proof verifies the exact ciphertext that will be stored and emitted. This implements the core PR objective of binding the vote to the proof.
176-176: LGTM: Event emits proof-verified ciphertext.The
InputPublishedevent correctly emitsvoteBytes(the ABI-encoded vote that was verified by the proof), maintaining the bytes format while ensuring authenticity.
181-198: LGTM: Vote processing updated with clearer parameter naming.The
_processVotefunction correctly uses theslotAddressparameter (now explicitly typed asaddress) for vote slot management. The first-vote vs re-vote logic remains sound.examples/CRISP/client/libs/crispWorker.js (5)
7-16: LGTM: Imports updated for proof generation workflow.The imports correctly include
generateProofand the newencodeSolidityProoffunction, aligning with the PR's shift to client-side proof encoding.
21-21: LGTM: Message type updated to reflect proof generation.The case statement correctly uses
'generate_proof'instead of'encrypt_vote', consistent with the renamed operation.
23-51: LGTM: Input preparation logic correctly preserved.The circuit input preparation (vote encoding, encryption, merkle proof generation) remains unchanged, which is correct since the circuit requirements haven't changed—only the output format has.
53-54: LGTM: Proof generation and Solidity encoding correctly implemented.The proof is generated and then encoded into Solidity ABI format using
encodeSolidityProof, which bundles the proof, vote ciphertext, and slot address into a single hex string that matches the contract's expected format.
56-63: LGTM: PostMessage payload correctly simplified.The success and error messages properly use the
'generate_proof'type, and the payload has been streamlined to send a singleencodedProofstring instead of a nested object, improving efficiency.
|
The convert_greco_coefficient_to_bfv function looks good |
YounesTal1
left a comment
YounesTal1
left a comment
There was a problem hiding this comment.
The conversion functions look good. Thanks
4 participants
The PR binds the circuit’s ciphertext output directly to the proof. Before this change, the ciphertext and the proof were disconnected, which made the proof effectively useless. Now the on-chain contract receives the exact ciphertext used during zk verification, encodes it, and emits it in the Ethereum event. The event type remains bytes, but the internal format is now the encoded (with
abi.encode) greco ciphertext, and needs proper conversion off-chain.On the CRISP program side, two new functions were added:
•
abi_decode_greco_ciphertext: performs ABI decoding (bytes → GRECO ciphertext),•
greco_to_bfv_ciphertext: converts the ciphertext from GRECO format to BFV format.These functions introduce additional cycles during the RISC0 verification, so checks were minimized and the implementations optimized.
This PR is the first step toward a transition where only the commitments of the circuit’s public inputs will be published on-chain, rather than the full polynomials. This means that instead of passing the ciphertext and the public-key row, only their hashes will be passed.
Closes #983
Summary by CodeRabbit
New Features
Changes
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.