refactor: make inputs public in CRISP circuit#995
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughThis PR refactors the CRISP voting system to remove Changes
Sequence DiagramsequenceDiagram
participant Server as Server
participant IndexerEvt as Indexer (E3Requested)
participant EVMHelper as EVM Helpers
participant Chain as Blockchain
Server->>IndexerEvt: E3Requested event received
IndexerEvt->>EVMHelper: CRISPContractFactory::create_write(rpc_url, contract, pk)
activate EVMHelper
EVMHelper->>EVMHelper: Build wallet signer & provider
EVMHelper-->>IndexerEvt: CRISPContract instance
deactivate EVMHelper
IndexerEvt->>EVMHelper: set_round_data(merkle_root, token_addr, threshold)
activate EVMHelper
EVMHelper->>Chain: Send setRoundData transaction
Chain-->>EVMHelper: TransactionReceipt
deactivate EVMHelper
EVMHelper-->>IndexerEvt: Receipt with tx hash
IndexerEvt->>Server: Log success with tx hash
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
cedoor
force-pushed
the
refactor/crisp-zk-pub-inputs
branch
from
3c62cbf to
5a9385c
Compare
cedoor
force-pushed
the
refactor/crisp-zk-pub-inputs
branch
from
5a9385c to
2dba7ad
Compare
cedoor
force-pushed
the
refactor/crisp-zk-pub-inputs
branch
from
2dba7ad to
05b0413
Compare
cedoor
force-pushed
the
refactor/crisp-zk-pub-inputs
branch
from
e068d3f to
2127059
Compare
cedoor
force-pushed
the
refactor/crisp-zk-pub-inputs
branch
from
2127059 to
ce1d284
Compare
cedoor
force-pushed
the
refactor/crisp-zk-pub-inputs
branch
from
ce1d284 to
2f2ba30
Compare
cedoor
changed the title
cedoor
force-pushed
the
refactor/crisp-zk-pub-inputs
branch
from
2f2ba30 to
81f74f5
Compare
cedoor
force-pushed
the
refactor/crisp-zk-pub-inputs
branch
from
237a313 to
4459bb5
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
🧹 Nitpick comments (1)
examples/CRISP/crates/evm_helpers/tests/integration.rs (1)
26-27: Derive the Anvil default key from the running instance instead of hard‑coding the literalAnvil's default private keys are intentionally well‑known/insecure for local development, and best practice is NOT to hard‑code those literal private keys into source. Instead, derive them from the running Anvil instance at runtime.
The code already has the infrastructure to do this:
setup_provider()(lines 13–21) already callsanvil.keys()[0]and returns theAnvilInstance. Modifysetup_provider()to also return the derived hex string ofanvil.keys()[0], then use it in all three test functions:
- Line 26 in
test_factory_creates_contract- Line 44 in
test_factory_invalid_address(The third test intentionally uses an invalid key, so it does not apply.)
This avoids hard‑coded literals triggering secret‑leak scanners and keeps tests aligned with the actual key Anvil is using.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/CRISP/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
examples/CRISP/Cargo.toml(2 hunks)examples/CRISP/circuits/src/main.nr(2 hunks)examples/CRISP/client/libs/wasm/pkg/crisp_worker.js(1 hunks)examples/CRISP/client/src/hooks/voting/useVoteCasting.ts(0 hunks)examples/CRISP/client/src/hooks/wasm/useWebAssembly.tsx(1 hunks)examples/CRISP/client/src/model/vote.model.ts(0 hunks)examples/CRISP/crates/evm_helpers/Cargo.toml(1 hunks)examples/CRISP/crates/evm_helpers/src/lib.rs(1 hunks)examples/CRISP/crates/evm_helpers/tests/integration.rs(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/tests/crisp.contracts.test.ts(0 hunks)examples/CRISP/server/Cargo.toml(1 hunks)examples/CRISP/server/src/server/indexer.rs(2 hunks)examples/CRISP/server/src/server/models.rs(0 hunks)examples/CRISP/server/src/server/routes/voting.rs(0 hunks)
💤 Files with no reviewable changes (5)
- examples/CRISP/server/src/server/models.rs
- examples/CRISP/server/src/server/routes/voting.rs
- examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts
- examples/CRISP/client/src/model/vote.model.ts
- examples/CRISP/client/src/hooks/voting/useVoteCasting.ts
🧰 Additional context used
🧠 Learnings (9)
📓 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-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/Cargo.tomlexamples/CRISP/server/Cargo.tomlexamples/CRISP/crates/evm_helpers/src/lib.rsexamples/CRISP/crates/evm_helpers/Cargo.toml
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Applied to files:
examples/CRISP/Cargo.toml
📚 Learning: 2025-10-04T14:18:45.636Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 660
File: crates/support-scripts/src/program_dev.rs:17-18
Timestamp: 2025-10-04T14:18:45.636Z
Learning: In crates/support-scripts/src/program_dev.rs, the #[allow(dead_code)] attribute on ProgramSupportDev is intentionally kept even though the struct appears to be referenced elsewhere, as confirmed by the maintainer.
Applied to files:
examples/CRISP/Cargo.toml
📚 Learning: 2024-10-22T02:36:01.448Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/data/Cargo.toml:0-0
Timestamp: 2024-10-22T02:36:01.448Z
Learning: In `packages/ciphernode/data/Cargo.toml`, `actix-rt` is only used in tests and should remain in `[dev-dependencies]`.
Applied to files:
examples/CRISP/Cargo.tomlexamples/CRISP/server/Cargo.toml
📚 Learning: 2024-11-05T03:56:48.126Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 170
File: packages/ciphernode/evm/tests/evm_reader.rs:79-80
Timestamp: 2024-11-05T03:56:48.126Z
Learning: In `packages/ciphernode/evm/tests/evm_reader.rs`, the `Anvil` instance is automatically cleaned up when its value is dropped, so explicit calls to `anvil.kill()` are unnecessary.
Applied to files:
examples/CRISP/crates/evm_helpers/tests/integration.rs
📚 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/crates/evm_helpers/src/lib.rsexamples/CRISP/server/src/server/indexer.rsexamples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol
📚 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-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/crates/evm_helpers/Cargo.toml
🧬 Code graph analysis (4)
examples/CRISP/crates/evm_helpers/tests/integration.rs (1)
examples/CRISP/crates/evm_helpers/src/lib.rs (3)
new(59-76)create_write(102-108)address(54-56)
examples/CRISP/client/src/hooks/wasm/useWebAssembly.tsx (1)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (1)
encryptedVote(38-38)
examples/CRISP/crates/evm_helpers/src/lib.rs (2)
crates/evm-helpers/src/listener.rs (1)
contract_address(99-99)crates/evm/src/helpers.rs (1)
provider(77-79)
examples/CRISP/server/src/server/indexer.rs (2)
examples/CRISP/crates/evm_helpers/src/lib.rs (1)
create_write(102-108)crates/evm-helpers/src/contracts.rs (1)
create_write(259-280)
🪛 Gitleaks (8.29.0)
examples/CRISP/crates/evm_helpers/tests/integration.rs
[high] 26-26: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 44-44: 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). (5)
- GitHub Check: crisp_e2e
- GitHub Check: rust_unit
- GitHub Check: integration_prebuild
- GitHub Check: build_sdk
- GitHub Check: rust_integration
🔇 Additional comments (13)
examples/CRISP/client/src/hooks/wasm/useWebAssembly.tsx (1)
46-51: LGTM! Simplified payload structure.The removal of
public_inputsandproofDatafrom the encrypted vote response aligns with the PR's objective to make circuit inputs public and simplify the vote payload structure.examples/CRISP/server/Cargo.toml (1)
47-47: LGTM! Proper dependency wiring.The local path dependency on the new
evm_helperscrate is correctly configured for server-side contract interactions.examples/CRISP/Cargo.toml (2)
8-8: LGTM! Workspace member registration.The new
crates/evm_helpersmember is properly registered in the CRISP workspace.
22-22: LGTM! Alloy dependency configuration.The workspace-level alloy dependency is correctly configured with pinned version and appropriate features for Ethereum contract interactions.
examples/CRISP/circuits/src/main.nr (1)
53-57: LGTM! Public visibility changes align with verifier expectations.Making
slot_addressandis_first_votepublic increases the circuit's public inputs from 0 to 2, which correctly matches the updated verifier contract'sNUMBER_OF_PUBLIC_INPUTS = 2. The TODO comments provide clear guidance for future parameter visibility changes.examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol (1)
10-16: Critical: Verify public inputs count matches circuit.The
NUMBER_OF_PUBLIC_INPUTShas been updated from 0 to 2, matching the circuit changes that madeslot_addressandis_first_votepublic. This is a critical change that affects proof verification. Ensure that:
- The circuit was recompiled after making these parameters public
- The verification key coordinates (lines 18-124) were regenerated using the updated circuit
- All proof generation code passes exactly 2 public inputs in the correct order
Based on learnings: The CRISP verifier contract coordinates are auto-generated and should match the compiled circuit's verification key.
examples/CRISP/crates/evm_helpers/Cargo.toml (1)
1-15: LGTM! Well-structured crate manifest.The new
evm_helperscrate is properly configured with:
- Workspace-level version, edition, and license inheritance
- Minimal, focused dependencies (alloy, eyre)
- Appropriate test dependencies with node-bindings for local testing
examples/CRISP/crates/evm_helpers/src/lib.rs (3)
24-30: LGTM! Contract interface definition.The
CRISPProgramcontract interface correctly defines thesetRoundDatafunction for on-chain Merkle root publishing.
45-95: LGTM! Solid contract wrapper implementation.The
CRISPContractimplementation follows best practices:
- Uses
Arcfor shared provider ownership in async contexts- Proper error handling with
eyre::Result- Clean separation between instantiation (
new) and contract interaction (set_round_data)- Correct parameter types matching the Solidity contract signature
97-109: LGTM! Factory pattern provides clean API.The
CRISPContractFactoryprovides a straightforward factory method for creating write-capable contract instances, encapsulating the complexity of provider setup.examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (1)
62-62: Code change is verified and correct.The
ProofDatatype from @aztec/bb.js has a nested.proofproperty structure, and the change at line 62 (proof: proof.proof) correctly extracts this nested property. This is confirmed by:
- Test files already using the
proof.proofpattern (e.g.,proof.proof.siblingsin utils.test.ts)generateProofWithReturnValuecorrectly returning aProofDataobject with this structure- The Aztec/Barretenberg library design where ProofData wraps the actual proof data
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol (1)
144-178: Public inputs alignment is correct; remove the stalecensusMerkleRootcommentVerification confirms the
validateInputimplementation is functionally sound:
- CRISPVerifier expects
publicInputsSizeof 2, which matches thenoirPublicInputsarray constructed invalidateInput.- The Noir circuit's public inputs are
slot_address: pub Fieldandis_first_vote: pub bool, in that order.validateInputcorrectly constructsnoirPublicInputs[0]as the slot andnoirPublicInputs[1]as the isFirstVote flag, matching the circuit's expectation.- Access control and round-data guards are appropriate.
The only actionable item is the commented line
// noirPublicInputs[x] = bytes32(roundData.censusMerkleRoot);—remove or replace it with a brief note about the future Merkle tree relocation to avoid confusing maintainers, sincenoirPublicInputsis now fixed at length 2 and the census root isn't included.examples/CRISP/server/src/server/indexer.rs (1)
32-32: Encoding assumptions and single-use contract semantics confirmed; clarify deployment modelVerification confirms all three concerns are real issues with the current implementation:
Merkle root encoding:
hex::decode(&merkle_root)at line 139 validates hex syntax but not length.U256::from_be_sliceat line 141 will silently truncate (if decoded bytes > 32) or left-pad (if < 32). Without explicit length validation before conversion, mismatches between Poseidon root format and U256 expectations could cause silent data corruption. Recommend validating decoded bytes are exactly 32 bytes before converting to U256.Balance threshold overflow:
balance_threshold.to_bytes_be()at line 144 converts unboundedBigUintto bytes with no check that the value fits in 256 bits.U256::from_be_slicewill truncate silently if > 32 bytes. Add an explicit bounds check or at minimum a debug log before conversion to catch misconfiguration early.Single-use
setRoundData+ global contract address:CONFIG.e3_program_addressis a global singleton (defined inconfig.rs:20). TheE3Requestedhandler always callssetRoundDataon the same contract instance. The Solidity contract enforces one-time-only via theRoundDataAlreadySetguard. This means the secondE3Requestedevent will fail completely. Verify your deployment model:
- If one CRISP program contract per census/E3, derive the program address from the event (not CONFIG) so each E3 gets its own instance.
- If truly one global contract, add a guard to check existing
roundDataon-chain and skip the write if already set, or fail fast with a clear operational error.
2 participants
Summary by CodeRabbit
Release Notes
New Features
Changes