chore: use merkle tree from server#1143
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
WalkthroughCircuit public inputs and client proof flow were updated to include Merkle data: the Noir circuit's Changes
Sequence DiagramsequenceDiagram
participant User as User/Client
participant VC as useVoteCasting Hook
participant ES as useEnclaveServer
participant Enclave as Enclave API
participant Worker as SDK Worker
participant Circuit as Noir Circuit
User->>VC: start castVoteWithProof
VC->>ES: getMerkleLeaves(round_id)
ES->>Enclave: POST /state/token-holders { round_id }
Enclave-->>ES: merkleLeaves (string[])
ES-->>VC: merkleLeaves
alt merkleLeaves missing/empty
VC-->>User: Error: MerkleLeaves unavailable
else
VC->>VC: convert merkleLeaves → BigInt[]
VC->>VC: handleProofGeneration(..., merkleLeaves)
VC->>Worker: postMessage({ ..., merkleLeaves })
Worker->>Circuit: generate proof (merkle_root now public input)
Circuit-->>Worker: proof + publicInputs (merkle_root, slotAddress, isFirstVote, vote...)
Worker->>Worker: encodeSolidityProof(publicInputs)
Worker-->>VC: encodedProof
VC-->>User: submitable proof
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🧬 Code graph analysis (1)examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (3)
⏰ 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)
🔇 Additional comments (3)
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: 1
🧹 Nitpick comments (2)
examples/CRISP/server/src/server/token_holders/etherscan.rs (1)
573-616: Verify that uniform mock balances provide adequate test coverage.All token holders now have a balance of "1", which simplifies the mock data but removes diversity that might be useful for testing balance-threshold logic or edge cases with varied voting power.
If the consuming tests need to validate behavior with different balance tiers, consider adding a few entries with varied balances (e.g., "1", "10", "100") to better represent real-world scenarios.
examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
283-284: Add documentation and consider named constants for public inputs indices.The public inputs structure is correctly aligned across the circuit, Solidity verifier, and SDK (merkleRoot at index 0, slotAddress at index 1, isFirstVote at index 2, vote array from index 3+). However, the hardcoded indices
1and3lack clarity:
- Add a comment above the function documenting the expected
publicInputsarray structure- Consider extracting indices as named constants (e.g.,
SLOT_ADDRESS_INDEX = 1,VOTE_START_INDEX = 3) for maintainability
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
examples/CRISP/circuits/src/main.nrexamples/CRISP/client/libs/crispSDKWorker.jsexamples/CRISP/client/src/context/voteManagement/VoteManagement.types.tsexamples/CRISP/client/src/hooks/enclave/useEnclaveServer.tsexamples/CRISP/client/src/hooks/voting/useSDKWorker.tsxexamples/CRISP/client/src/hooks/voting/useVoteCasting.tsexamples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.solexamples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.solexamples/CRISP/packages/crisp-sdk/src/vote.tsexamples/CRISP/server/src/server/token_holders/etherscan.rs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
📚 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.solexamples/CRISP/circuits/src/main.nrexamples/CRISP/client/libs/crispSDKWorker.js
📚 Learning: 2025-12-19T11:35:43.204Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 1109
File: examples/CRISP/server/src/server/routes/state.rs:33-37
Timestamp: 2025-12-19T11:35:43.204Z
Learning: In the CRISP voting system (examples/CRISP), ciphertext data and slot states are public and stored on-chain in the CRISPProgram smart contract. The server endpoints `/previous-ciphertext` and `/is-slot-empty` expose public blockchain data that can already be accessed by anyone through the contract's external view functions (getSlotIndex, isSlotEmptyByAddress) or by reading contract state directly. Authentication is not needed for these endpoints since they only expose data that is already publicly available on the blockchain.
Applied to files:
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build_sdk
- GitHub Check: rust_integration
- GitHub Check: build_enclave_cli
- GitHub Check: rust_unit
- GitHub Check: build_crisp_sdk
- GitHub Check: build_e3_support_dev
- GitHub Check: integration_prebuild
- GitHub Check: test_net
- GitHub Check: Build & Push Image
🔇 Additional comments (11)
examples/CRISP/circuits/src/main.nr (1)
77-77: LGTM! Merkle root is now a public input.The change correctly exposes the merkle root as a public input to the circuit, enabling the verifier to validate it against the expected value stored in the contract.
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol (3)
56-56: LGTM! New error for merkle root validation.The new error provides clear feedback when attempting to validate input before the merkle root is set.
127-127: LGTM! Merkle root presence enforced.The check ensures that the merkle root must be set before any input validation, preventing votes from being submitted without a valid census.
138-145: LGTM! Public inputs array correctly updated.The array construction now includes the merkle root as the first element (index 0), followed by slotAddress, isFirstVote, and vote data. This matches the updated Noir circuit where merkle_root is now a public input.
examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol (1)
10-11: LGTM! Verifier constants updated for expanded public inputs.The verifier correctly reflects the addition of one public input (merkle_root), with NUMBER_OF_PUBLIC_INPUTS incremented from 2066 to 2067 and the verification key hash updated accordingly.
Also applies to: 17-17
examples/CRISP/client/src/hooks/voting/useSDKWorker.tsx (1)
31-53: LGTM! Merkle leaves propagated to worker.The function signature correctly includes the merkleLeaves parameter and passes it through to the worker message data.
examples/CRISP/client/src/context/voteManagement/VoteManagement.types.ts (1)
40-50: LGTM! Type signature updated for merkle leaves.The generateProof signature correctly includes merkleLeaves as the final parameter, maintaining consistency with the implementation.
examples/CRISP/client/src/hooks/enclave/useEnclaveServer.ts (1)
32-32: LGTM! Server endpoint for merkle leaves added.The new getMerkleLeaves method follows the established pattern for fetching data from the enclave server and is correctly exposed in the hook's return value.
Also applies to: 46-47, 58-58
examples/CRISP/client/libs/crispSDKWorker.js (1)
7-7: LGTM! Worker now consumes merkle leaves from input.The worker correctly receives merkleLeaves from the data payload and passes them to both generateMaskVoteProof and generateVoteProof methods, removing the need for local leaf computation.
Also applies to: 14-39
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (2)
82-104: LGTM! Proof generation updated to include merkle leaves.The handleProofGeneration function correctly accepts merkleLeaves and passes them through to generateProof, maintaining the callback pattern used throughout the hook.
248-248: The merkle leaves hex format is handled correctly. The server returns unprefixed hex strings (verified by the test case showing"0cb36cd64fcc99d7f742ae77954eda75236e182d7c10de1660f62f56c582b518"), and the client code inuseVoteCasting.tsappropriately adds the "0x" prefix before converting to BigInt. No "0x0x" format issue exists.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (1)
331-331: AddgetMerkleLeavesto the dependency array.The
castVoteWithProofcallback usesgetMerkleLeaves(line 235) but doesn't include it in the dependency array. This violates React's exhaustive-deps rule and could lead to stale closure bugs.Following the pattern established in this file (e.g.,
getEligibleVotersis included in line 149),getMerkleLeavesshould also be included.🔎 Recommended fix
[user, roundState, broadcastVote, setTxUrl, showToast, navigate, handleProofGeneration, markVotedInRound, handleMask, handleVote], + [user, roundState, broadcastVote, setTxUrl, showToast, navigate, handleProofGeneration, markVotedInRound, handleMask, handleVote, getMerkleLeaves],When a value referenced inside these hooks isn't included in the dependency array, React won't re-run the effect or recalculate the value when that dependency changes. This causes stale closures where the hook uses outdated values.
♻️ Duplicate comments (1)
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (1)
235-239: Strengthen merkle leaves validation.The optional chaining operator in the length check is redundant after the nullish check. This was previously flagged.
🔎 Recommended fix (as per previous review)
- if (!merkleLeaves || merkleLeaves?.length === 0) { + if (!merkleLeaves || merkleLeaves.length === 0) { throw new Error('No merkle leaves available for proof generation') }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
🧬 Code graph analysis (1)
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (3)
examples/CRISP/client/src/hooks/enclave/useEnclaveServer.ts (1)
useEnclaveServer(35-60)examples/CRISP/client/src/model/vote.model.ts (1)
Vote(59-62)examples/CRISP/packages/crisp-sdk/src/types.ts (1)
Vote(70-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). (9)
- GitHub Check: rust_unit
- GitHub Check: build_crisp_sdk
- GitHub Check: test_net
- GitHub Check: build_e3_support_dev
- GitHub Check: build_sdk
- GitHub Check: build_enclave_cli
- GitHub Check: rust_integration
- GitHub Check: integration_prebuild
- GitHub Check: Build & Push Image
🔇 Additional comments (3)
examples/CRISP/client/src/hooks/voting/useVoteCasting.ts (3)
72-72: LGTM!The addition of
getMerkleLeavesfrom the enclave server hook is consistent with the PR objective and properly integrates with the existing pattern.
82-105: LGTM!The signature extension to include
merkleLeavesis implemented correctly. The parameter is properly threaded through to thegenerateProofcall, and theuseCallbackdependency array correctly omits it (parameters don't need to be included).
241-249: LGTM!The conversion of merkle leaves from hex strings to
BigIntis correctly implemented. The validation at lines 237-239 ensures the array is defined and non-empty before this operation.
2 participants
pass merkle root as public input and get the merkle leaves from the server
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.