refactor: crisp sdk [skip-line-limit]#1055
Conversation
WalkthroughRefactors CRISP SDK types/exports and vote/proof APIs, updates client worker/hooks to new proof payload, changes circuit merkle-root/address gating and adds runtime assertion, replaces verifier contract verification key, and updates tests, docs, and package versions to align with the new API. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ 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.
1 Skipped Deployment
|
cedoor
force-pushed
the
refactor/crisp-sdk
branch
from
086d607 to
1601317
Compare
Use os.availableParallelism() to automatically determine optimal thread count for proof generation, leaving at least 1 core free for other operations. This adapts to different hardware configurations instead of hardcoding 4 threads.
- Extract witness generation into separate generateWitness function - Remove unused generateProofWithReturnValue and getCircuitOutputValue - Update tests to use proof.publicInputs instead of returnValue - Remove unused test utilities (utils.ts)
- Updated @crisp-e3/sdk to 0.4.0 - Updated @crisp-e3/contracts to 0.4.0 - Updated @crisp-e3/zk-inputs to 0.4.0 - 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
📜 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 (7)
examples/CRISP/client/libs/crispWorker.js(1 hunks)examples/CRISP/client/package.json(1 hunks)examples/CRISP/packages/crisp-contracts/package.json(1 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(3 hunks)examples/CRISP/packages/crisp-sdk/package.json(2 hunks)examples/CRISP/packages/crisp-sdk/src/index.ts(1 hunks)examples/CRISP/packages/crisp-zk-inputs/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- examples/CRISP/packages/crisp-zk-inputs/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/CRISP/packages/crisp-sdk/package.json
🧰 Additional context used
🧠 Learnings (4)
📓 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-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-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/tests/crisp.contracts.test.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/packages/crisp-contracts/tests/crisp.contracts.test.ts
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (2)
examples/CRISP/packages/crisp-sdk/src/vote.ts (3)
generatePublicKey(89-91)generateVoteProof(141-166)encodeSolidityProof(197-202)examples/CRISP/packages/crisp-sdk/src/utils.ts (3)
getAddressFromSignature(119-123)hashLeaf(20-22)generateMerkleTree(29-31)
⏰ 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: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: test_net
- GitHub Check: test_contracts
- GitHub Check: crisp_rust_unit
- GitHub Check: build_ciphernode_image
- GitHub Check: rust_unit
- GitHub Check: rust_integration
🔇 Additional comments (6)
examples/CRISP/packages/crisp-contracts/package.json (1)
3-3: Version bump aligns with coordinated CRISP release.The version update to 0.4.0 is appropriate for this refactor release. All three main CRISP packages (@crisp-e3/contracts, @crisp-e3/sdk, @crisp-e3/zk-inputs) are coordinated at this version, consistent with the breaking SDK API changes, new proof generation functions, and circuit updates described in the PR objectives.
examples/CRISP/packages/crisp-sdk/src/index.ts (1)
9-21: Explicit exports improve API clarity; past concern addressed.The refactor to explicit named exports clearly defines the public SDK surface. The previously flagged missing
encryptVoteexport is now included on line 17. The selective exposure pattern (explicit exports for constants/utils/vote vs.export *for token/state) effectively hides internal implementation details from SDK consumers.examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (3)
8-20: Test refactoring correctly aligns with the new SDK API.The imports and usage patterns correctly adopt the refactored SDK surface. The flow of deriving the address from the signature and using it to construct Merkle leaves aligns with the PR objectives of streamlining proof generation inputs.
63-82: Test validates the consolidated proof generation flow.The test correctly exercises the new
generateVoteProofAPI with signature-derived address, Merkle leaves construction, and balance validation. The proof verification against the honk verifier confirms the circuit integration works end-to-end.
84-116: Integration test correctly validates contract-level input verification.The test properly constructs the Merkle tree, sets the round data with the tree root, and validates the encoded proof against the contract. This confirms the refactored SDK integrates correctly with the contract validation logic.
examples/CRISP/client/package.json (1)
21-21: @crisp-e3/sdk@0.4.0 API is compatible with all client usagesAll functions used in the CRISP client (
hashLeaf,generateVoteProof,encodeSolidityProof,SIGNATURE_MESSAGE) are properly exported from the@crisp-e3/sdk@0.4.0package and match the existing implementation. No API changes are required.
- Updated @crisp-e3/sdk to 0.4.1 - Updated @crisp-e3/contracts to 0.4.1 - Updated @crisp-e3/zk-inputs to 0.4.1 - Published to npm
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
examples/CRISP/packages/crisp-sdk/src/utils.ts (1)
130-149: Consider adding a minimum bound for the fallback value.The fallback of
5threads is reasonable, but consider documenting why this specific value was chosen. Also, the function could benefit from a configurable maximum to prevent over-allocation on high-core-count systems.+ // Fallback: conservative default for unknown environments return 5examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
131-140: Backend instance is not reused across calls.Each call to
generateProofcreates a newUltraHonkBackendinstance, which may have initialization overhead. For batch proof generation, consider exposing a way to reuse the backend.This is acceptable for the current API design focused on simplicity, but for performance-critical use cases, consider documenting this behavior or providing a lower-level API for backend reuse.
📜 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 (7)
examples/CRISP/client/package.json(1 hunks)examples/CRISP/packages/crisp-contracts/package.json(1 hunks)examples/CRISP/packages/crisp-sdk/package.json(2 hunks)examples/CRISP/packages/crisp-sdk/src/constants.ts(2 hunks)examples/CRISP/packages/crisp-sdk/src/utils.ts(3 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(1 hunks)examples/CRISP/packages/crisp-zk-inputs/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/CRISP/client/package.json
- examples/CRISP/packages/crisp-sdk/src/constants.ts
- examples/CRISP/packages/crisp-sdk/package.json
- examples/CRISP/packages/crisp-zk-inputs/package.json
🧰 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/packages/crisp-sdk/src/utils.ts (3)
examples/CRISP/packages/crisp-sdk/src/index.ts (4)
hashLeaf(10-10)generateMerkleProof(10-10)generateMerkleTree(10-10)getAddressFromSignature(10-10)examples/CRISP/packages/crisp-sdk/src/types.ts (1)
MerkleProof(59-65)examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
SIGNATURE_MESSAGE_HASH(31-31)
⏰ 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: test_net
- GitHub Check: integration_prebuild
- GitHub Check: build_sdk
- GitHub Check: test_contracts
- GitHub Check: build_ciphernode_image
- GitHub Check: build_enclave_cli
- GitHub Check: crisp_rust_unit
- GitHub Check: rust_unit
- GitHub Check: rust_integration
🔇 Additional comments (12)
examples/CRISP/packages/crisp-contracts/package.json (1)
3-3: Version bump to 0.4.1 is consistently applied across all CRISP packages.All three CRISP packages (
@crisp-e3/contracts,@crisp-e3/sdk,@crisp-e3/zk-inputs) have been updated to version 0.4.1, confirming coordinated versioning across the workspace.examples/CRISP/packages/crisp-sdk/src/utils.ts (4)
20-22: LGTM! Address normalization for consistent hashing.The lowercase normalization ensures consistent leaf hashing regardless of input address case (checksummed vs lowercase).
52-68: Merkle proof padding logic looks correct.The sibling padding to
MERKLE_TREE_MAX_DEPTHwith0nand index extraction via bit manipulation are appropriate for fixed-depth circuit inputs.
89-117: LGTM! Clean signature component extraction for Noir circuit.The function correctly recovers the public key from the signature, extracts X/Y coordinates (skipping the 0x04 prefix byte), and concatenates R and S components. This aligns with secp256k1 signature verification requirements.
119-123: LGTM! Address recovery utility.Clean helper that derives the signer address from a signature using the hardcoded message hash, aligning with the PR objective to remove runtime message hash parameters.
examples/CRISP/packages/crisp-sdk/src/vote.ts (7)
26-47: LGTM! Vote encoding logic is clear and correct.The binary encoding correctly pads yes/no values into the first and second halves of the degree-length array.
54-82: LGTM! Tally decoding correctly extracts binary values.The extraction of relevant slices and binary-to-bigint conversion is correct.
142-167: Vote validation and proof generation look correct.The validation checks are appropriate:
- Vote values don't exceed balance
- Vote values don't exceed maximum allowed
- Vote values are non-negative
The address is correctly derived from the signature for real votes (vs mask votes).
182-190: LGTM! Proof verification correctly mirrors generation.Uses the same backend configuration with
keccakZK: truefor consistency with proof generation.
17-19: No action required. The SDK is configured as ES modules only ("type": "module") with no CommonJS output, and its primary consumer already includesvite-plugin-top-level-awaitto handle top-level await. Top-level await at lines 17-19 is appropriate for this ESM-only configuration.Likely an incorrect or invalid review comment.
169-180: The constantMASK_SIGNATUREis properly defined inconstants.tsas a valid hex-formatted signature placeholder (0x8e7d77...) explicitly documented with the comment "Placeholder signature for masking votes." This is intentional design—mask votes use zeroed vote values with a constant placeholder signature rather than requiring actual signature verification. No action needed.
94-121: Circuit properly enforces theis_first_voteconstraint—no security issue.The circuit correctly handles the
is_first_voteflag: when true, it skips the ciphertext addition verification and outputs the encrypted vote directly; when false, it requires the ciphertext addition to be valid. Tests confirm both paths work correctly, including the case wherepreviousCiphertextis omitted and a placeholder is generated.
Co-authored-by: Giacomo <hello@0xjei.dev>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
examples/CRISP/packages/crisp-sdk/src/utils.ts (1)
39-68: Merkle proof generation looks coherent; consider normalizing leaves once and avoiding redundant lowercasing.The flow
leaf -> index lookup -> tree -> proof paddingis internally consistent, and theBigInt(l)normalization in bothfindIndexandgenerateMerkleTreefixes the earlier type mismatch.Two small cleanups you may want to consider:
hashLeafalready lowercases the address, sohashLeaf(address, balance)here is enough and keeps normalization in a single place.- Since you call
BigInt(l)both infindIndexand before tree creation, you could map once, e.g.const normalizedLeaves = leaves.map(l => BigInt(l)), then usenormalizedLeavesfor both index search andgenerateMerkleTreeto avoid double work and ensure all downstream consumers share exactly the same bigint view.
🧹 Nitpick comments (3)
examples/CRISP/packages/crisp-sdk/README.md (1)
20-72: Usage section is well-structured but lacks type guidance.The usage examples follow a good pattern (import → usage) and cover all main refactored functions. However, examples would benefit from documenting parameter types (e.g., is
publicKeyBytesaUint8Arrayor hex string? what format formerkleLeaves?). As the docs currently stand, developers must infer types from context, which can lead to integration errors.This is a nice-to-have improvement for developer experience.
examples/CRISP/packages/crisp-sdk/src/utils.ts (2)
84-117: Signature component extraction matches expected layouts; add basic length validation to harden against malformed inputs.Given current viem behavior, slicing the recovered public key into
[1,33)and[33,65)and the signature bytes into[0,32)and[32,64)is consistent, and the returned structure is suitable for a Noir circuit.To make this more robust and easier to debug if inputs are wrong or viem’s formats ever change, you might:
- Assert
publicKeyBytes.length === 65before slicing, and throw a clear error otherwise.- Assert
sigBytes.length >= 65before buildingr/s, instead of silently truncating.This keeps failures explicit at the boundary instead of propagating subtly corrupted data into the circuit.
119-123:getAddressFromSignatureis fine; consider centralizing therecoverPublicKeylogic.The function correctly derives the signer address from the fixed
SIGNATURE_MESSAGE_HASHusingrecoverPublicKeyandpublicKeyToAddress. To reduce duplication and keep all assumptions about viem’s key format in one place, you could reuse a shared helper (orextractSignatureComponents’s recovered public key) rather than callingrecoverPublicKeyin two separate functions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/CRISP/packages/crisp-sdk/README.md(1 hunks)examples/CRISP/packages/crisp-sdk/src/utils.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 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-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/README.md
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-sdk/src/utils.ts (3)
examples/CRISP/packages/crisp-sdk/src/index.ts (4)
hashLeaf(10-10)generateMerkleProof(10-10)generateMerkleTree(10-10)getAddressFromSignature(10-10)examples/CRISP/packages/crisp-sdk/src/types.ts (1)
MerkleProof(59-65)examples/CRISP/packages/crisp-sdk/src/constants.ts (1)
SIGNATURE_MESSAGE_HASH(31-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build_sdk
- GitHub Check: build_enclave_cli
- GitHub Check: integration_prebuild
- GitHub Check: test_net
- GitHub Check: rust_integration
- GitHub Check: test_contracts
- GitHub Check: crisp_rust_unit
- GitHub Check: rust_unit
🔇 Additional comments (4)
examples/CRISP/packages/crisp-sdk/README.md (3)
57-63: Example parameters are accurate.The
generateMaskVoteProoffunction accepts aMaskVoteProofInputsobject with the parameters shown:balance(bigint),slotAddress(string),publicKey(Uint8Array),merkleLeaves(string[] | bigint[]), and optionalpreviousCiphertext(Uint8Array). The example matches the actual function signature.
74-79: All listed functions are correctly exported from the module.Verification confirms that all 12 functions listed in the API reference (State, Token, Vote, and Utils modules) are properly exported from
@crisp-e3/sdkin the main index.ts file. The module exports match the developer-intended public API documented in the README.Note: The module also exports three additional Vote functions not mentioned in the README (
generatePublicKey,encryptVote,encodeSolidityProof). Consider whether these should be included in the API documentation.
43-49: No issues found. ThegenerateVoteProofexample parameters (vote,publicKey,signature,balance,merkleLeaves) accurately match theVoteProofInputstype definition in the refactored SDK.examples/CRISP/packages/crisp-sdk/src/utils.ts (1)
130-149: Environment‑aware thread count helper looks good.The Node/browser detection, use of
availableParallelism/cpuswith a “leave one core free” policy, browserhardwareConcurrencyhandling, and constant fallback make this a reasonable and robust default for proof generation workloads.
2 participants
Closes #988
The focus of this PR is improving the developer experience of the CRISP JS SDK, specifically around proof generation and verification. It introduces three main functions:
generateVoteProof,generateMaskVoteProof, andverifyProof. The inputs required for generating proofs have been streamlined to minimize the chance of developer errors.Main changes:
Read the SDK README file to see some usage examples
Note
What makes the limit of lines of code per PR exceed is the 800 lines of code removed with
ERC20Votes.json.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.