feat: move wasm-crypto to the sdk#635
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@ctrlc03 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughAdds parameterized BFV encryption and a verifiable encryption path (Greco inputs) in Rust, exposes them via wasm exports, integrates BFV protocol support into the TypeScript Enclave SDK (class API), removes the CRISP wasm-crypto crate and client wasm bindings, and updates examples, tests, packaging, and CI accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant SDK as EnclaveSDK (TS)
participant Init as wasm/init (node/web)
participant WASM as crates/wasm (BFV exports)
participant Helpers as bfv-helpers (Rust)
participant Greco as greco / Noir backend
App->>SDK: encryptNumber(value, publicKey, params)
SDK->>Init: initializeWasm()
SDK->>WASM: bfv_encrypt_number(value, pk, degree, t, q)
WASM->>Helpers: bfv_encrypt_u64(...)
Helpers-->>WASM: ciphertext bytes
WASM-->>SDK: Uint8Array
SDK-->>App: Uint8Array
App->>SDK: encryptNumberAndGenProof(value, publicKey, circuit, params)
SDK->>Init: initializeWasm()
SDK->>WASM: bfv_verifiable_encrypt_number(value, pk, degree, t, q)
WASM->>Helpers: bfv_verifiable_encrypt_u64(...)
Helpers->>Greco: InputValidationVectors -> circuit_inputs JSON
Helpers-->>WASM: [ciphertext, circuit_inputs]
SDK->>Greco: generateProof(circuit_inputs, circuit)
Greco-->>SDK: ProofData
SDK-->>App: { encryptedVote, proof }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🔭 Outside diff range comments (6)
examples/CRISP/apps/server/src/server/mod.rs (1)
57-63: CORS misconfiguration: allow_any_origin with credentials will break and violates the spec.actix-cors disallows using allow_any_origin together with supports_credentials. This can panic at runtime and, even if it didn’t, browsers won’t accept Access-Control-Allow-Origin: * with credentials.
Apply one of the following diffs:
- If you do not need credentials, just drop supports_credentials:
- let cors = Cors::default() - .allow_any_origin() - .allowed_methods(vec!["GET", "POST", "OPTIONS"]) - .allow_any_header() - .supports_credentials() - .max_age(3600); + let cors = Cors::default() + .allow_any_origin() + .allowed_methods(vec!["GET", "POST", "OPTIONS"]) + .allow_any_header() + .max_age(3600);
- If you do need credentials, restrict origins explicitly (example):
- let cors = Cors::default() - .allow_any_origin() + let cors = Cors::default() + .allowed_origin("http://localhost:3000") .allowed_methods(vec!["GET", "POST", "OPTIONS"]) .allow_any_header() .supports_credentials() .max_age(3600);Consider pulling allowed origins from CONFIG to avoid hardcoding.
examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx (1)
33-41: Avoid race conditions by not overwriting worker.onmessage per callOverwriting worker.onmessage per invocation breaks concurrency: parallel encryptVote calls will clobber each other’s handlers. Prefer addEventListener with { once: true } and filter by message type or a requestId.
Minimal change without request IDs:
- worker.postMessage({ type: 'encrypt_vote', data: { voteId, publicKey } }) - worker.onmessage = async (event) => { + const onMessage = async (event: MessageEvent) => { const { type, success, encryptedVote, error } = event.data if (type === 'encrypt_vote') { if (success) { const { vote, proofData } = encryptedVote; const { proof, publicInputs } = proofData - resolve({ vote: vote, proof: proof, public_inputs: publicInputs, }) } else { showToast({ type: 'danger', message: error, }) handleGenericError('encryptVote', new Error(error)) reject(new Error(error)) } setIsLoading(false) + worker?.removeEventListener('message', onMessage) } - } + } + worker.addEventListener('message', onMessage, { once: false }) + worker.postMessage({ type: 'encrypt_vote', data: { voteId, publicKey } })If multiple message types are used by the worker, add a requestId to correlate responses to requests.
packages/enclave-react/src/useEnclaveSDK.ts (1)
108-111: Re-initialization misses changes to protocol/params; avoid stalesdkvia ref
- The
initializeSDKcallback doesn’t depend onconfig.protocol/config.protocolParams, so changes to either won’t trigger re-init.- The callback closes over
sdk; prefersdkRef.currentto avoid stale closures.Apply:
- if (sdk) { - sdk.cleanup(); - } + if (sdkRef.current) { + sdkRef.current.cleanup(); + }And include the new deps:
- }, [publicClient, walletClient, config.contracts, config.chainId]); + }, [ + publicClient, + walletClient, + config.contracts, + config.chainId, + config.protocol, + config.protocolParams, + ]);This also ensures the effect that depends on
initializeSDKwill re-run when these change.Also applies to: 137-137
crates/wasm/README.md (3)
3-3: Fix typo: “funcionality” → “functionality”; “Typescript” → “TypeScript”Minor doc polish for clarity and professionalism.
-Here we export wasm funcionality for consumption in typescript to enable use to share code between Rust and Typescript. +Here we export Wasm functionality for consumption in TypeScript to enable us to share code between Rust and TypeScript.
11-11: Typo in headingUse “DON’T” for consistency with the affirmative heading.
-##### ❌ DONT USE THE DEFAULT INIT +##### ❌ DON’T USE THE DEFAULT INIT
21-24: Import the new export:bfv_encrypt_numberThe example imports
encrypt_numberbut then callsbfv_encrypt_number. Update the import to match the call.-// Good! Use the universal loader -import init from "@gnosis-guild/e3-wasm/init"; -import { encrypt_number } from "@gnosis-guild/e3-wasm"; +// Good! Use the universal loader +import init from "@gnosis-guild/e3-wasm/init"; +import { bfv_encrypt_number } from "@gnosis-guild/e3-wasm";
🧹 Nitpick comments (43)
examples/CRISP/tsconfig.json (3)
8-9: tsBuildInfoFile under dist may be wiped by clean buildsPlacing the build info inside dist can force a full rebuild after a clean, losing the benefits of incremental builds. Consider keeping it at project root.
Apply this diff:
- "tsBuildInfoFile": "./dist/.tsbuildinfo", + "tsBuildInfoFile": "./.tsbuildinfo",
6-7: Prefer NodeNext resolution in examples/CRISP/tsconfig.jsonThe CRISP example depends on ESM-only packages (ethers v6 and viem), so TypeScript’s legacy “node” resolver may fail to honor their package.json “exports” fields. Update your tsconfig to use the newer “nodenext” resolver:
Apply this diff in
examples/CRISP/tsconfig.json:--- a/examples/CRISP/tsconfig.json +++ b/examples/CRISP/tsconfig.json @@ - "module": "commonjs", - "moduleResolution": "node", + "module": "commonjs", + "moduleResolution": "nodenext",This change preserves CommonJS output while ensuring correct resolution of modern ESM/“exports”-based dependencies.
3-4: Expand TSConfig include globs to cover all TS/TSX filesA quick scan shows you currently only include
tasks,test,deploy, andtypes, but there are many.ts/.tsxfiles underapps/clientand root-level config files (e.g.hardhat.config.ts,playwright.config.ts) that aren’t being picked up. To ensure your compiler and IDE see everything, consider widening the globs:"include": [ - "tasks/**/*", - "test/**/*", - "deploy/**/*", - "types/**/*" + "tasks/**/*", + "test/**/*", + "deploy/**/*", + "types/**/*", + "apps/**/*", + "*.ts", + "*.tsx" ]This will cover all TS/TSX under
apps/as well as any top-level.ts/.tsxfiles.examples/CRISP/contracts/CRISPVerifier.sol (3)
480-496: Transcript round0 now appends pairingPointObject after “regular” public inputsThis preserves ordering across transcript, the public-input delta, and the encoding, while keeping calldata small by not requiring the 16 pairing elements be passed in publicInputs. One nit: the huge temporary array allocation is gas-expensive.
Two improvements:
- Avoid allocating the full round0 array by hashing incrementally.
- Micro-opt: use unchecked ++i loops to shave gas.
Example (conceptual; not a strict diff because it spans multiple lines):
- bytes32[] memory round0 = new bytes32[](3 + publicInputsSize + 12); - round0[0] = bytes32(circuitSize); - round0[1] = bytes32(publicInputsSize); - round0[2] = bytes32(pubInputsOffset); - for (uint256 i = 0; i < publicInputsSize - PAIRING_POINTS_SIZE; i++) { - round0[3 + i] = bytes32(publicInputs[i]); - } - for (uint256 i = 0; i < PAIRING_POINTS_SIZE; i++) { - round0[3 + publicInputsSize - PAIRING_POINTS_SIZE + i] = FrLib.toBytes32(proof.pairingPointObject[i]); - } - // then round0[...] = w1/w2/w3 limbs - previousChallenge = FrLib.fromBytes32(keccak256(abi.encodePacked(round0))); + bytes32 h = keccak256( + abi.encodePacked(bytes32(circuitSize), bytes32(publicInputsSize), bytes32(pubInputsOffset)) + ); + // hash public inputs + for (uint256 i; i < publicInputsSize - PAIRING_POINTS_SIZE; ) { + h = keccak256(abi.encodePacked(h, publicInputs[i])); + unchecked { ++i; } + } + // hash pairing point object + for (uint256 i; i < PAIRING_POINTS_SIZE; ) { + h = keccak256(abi.encodePacked(h, FrLib.toBytes32(proof.pairingPointObject[i]))); + unchecked { ++i; } + } + // hash w1/w2/w3 limbs + h = keccak256( + abi.encodePacked( + h, + proof.w1.x_0, proof.w1.x_1, proof.w1.y_0, proof.w1.y_1, + proof.w2.x_0, proof.w2.x_1, proof.w2.y_0, proof.w2.y_1, + proof.w3.x_0, proof.w3.x_1, proof.w3.y_0, proof.w3.y_1 + ) + ); + previousChallenge = FrLib.fromBytes32(h);If you prefer to keep the array approach for clarity, consider just using
unchecked { ++i; }in both loops.
680-707: Proof loader updated to read pairingPointObject first — ordering matches challenge derivation
- Reads 16 Frs for pairingPointObject first, then w1/w2/w3, then lookup commits, w4, inverses, zPerm, etc.
- This ordering aligns with generateEtaChallenge (w1-3 in round0, lookup + w4 used in round1) and the new overall proof layout.
Add a sanity check at the end of loadProof to catch trailing/short proofs early:
function loadProof(bytes calldata proof) internal pure returns (Honk.Proof memory p) { // ... p.kzgQuotient = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + require(boundary + 0x80 == proof.length, "proof length mismatch"); }Also consider
unchecked { ++i; }in the small for-loop to marginally reduce gas in hot paths.
485-517: TODO notes on “publicInputsSize not include pairing point object”The current approach keeps publicInputsSize inclusive of pairing points, while calldata publicInputs excludes them. This works (you account for PAIRING_POINTS_SIZE where needed), but it’s easy to misunderstand later.
Consider either:
- Renaming
publicInputsSizetopublicInputsPlusPairingSize, or- Storing a second field in VK for
publicInputsSizeExcludingPairingto avoid the repeated subtraction and TODOs.examples/CRISP/apps/server/src/server/mod.rs (3)
49-51: Use logger instead of eprintln! for consistency.Since the logger is initialized, prefer log::error! so messages are structured and filtered consistently.
- eprintln!("Listener failed: {:?}", e); + log::error!("Listener failed: {:?}", e);
72-72: Prefer log::info! over println!.Keeps output consistent with the configured logger and middleware logger.
- println!("'crisp-server' listening on http://{}", bind_addr); + log::info!("'crisp-server' listening on http://{}", bind_addr);
30-30: Logger init may panic if called twice.env_logger::Builder::init() panics if the logger is already set. If start() can be invoked multiple times (e.g., in tests), consider try_init to avoid panic.
- init_logger(); + if let Err(_) = env_logger::Builder::new().try_init() { + // already initialized; ignore + }Alternatively, change init_logger() to use try_init internally.
examples/CRISP/.env.example (1)
1-3: Tighten .env template: remove quotes, add missing INFURA_API_KEY, and clarify key usage.Aligns with dotenv-linter hints, avoids quoting pitfalls, and makes required envs explicit. Also ensures a trailing newline.
-PRIVATE_KEY="" -MNEMONIC="" -RPC_URL="http://localhost:8545" +# One of PRIVATE_KEY or MNEMONIC should be set. If both are set, PRIVATE_KEY takes precedence. +MNEMONIC= +PRIVATE_KEY= +RPC_URL=http://localhost:8545 +INFURA_API_KEY=crates/evm-helpers/src/contracts.rs (1)
385-388: Remove hardcoded gas limit inpublishInputbuilderValidation via
rg -nP '\.gas\s*\(' -C2confirms this is the only manual.gas(…)override in the codebase. Hardcoding.gas(10_000_000)bypasses the GasFiller estimator and can lead to transaction failures on some networks or unnecessary gas waste. To keep behavior consistent with other contract calls (e.g.,request_e3,activate):• crates/evm-helpers/src/contracts.rs:388
Suggested change:
- let builder = contract - .publishInput(e3_id, data) - .nonce(nonce) - .gas(10_000_000); + let builder = contract + .publishInput(e3_id, data) + .nonce(nonce);If you’ve seen estimator underestimation on large calldata, introduce a configurable override (e.g., via your CONFIG) or apply an “estimate + headroom” multiplier rather than a fixed constant.
crates/wasm/init_node.js (1)
7-7: Keep signature parity with web initializer.Accepting an unused parameter maintains API symmetry with initializeWasm(initParams) on the web path and avoids potential type-level confusion.
-export default async function initializeWasm() { +export default async function initializeWasm(_initParams) { // Node does not need to be loaded async }crates/wasm/init_web.js (1)
8-10: Minor: unnecessary await.Returning the promise directly is marginally cleaner.
-export default async function initializeWasm(initParams) { - return await init(initParams); -} +export default async function initializeWasm(initParams) { + return init(initParams); +}crates/wasm/init_node.cjs (1)
7-9: Ensure CJS/ESM interop by explicitly setting a default export on module.exportsSome bundlers/transpilers expect a
.defaultproperty when consuming a CJS module via ESM-style default imports. While esbuild often wraps this automatically, adding it here removes ambiguity and avoids surprises across toolchains.Apply this diff:
module.exports = async function initializeWasm() { // Node does not need to be loaded async } +// Ensure default export interop for ESM consumers importing the CJS file +module.exports.default = module.exportspackages/enclave-sdk/tsup.config.js (1)
24-36: Avoid generating DTS twice across dual buildsBoth entries spread ...baseConfig, which has dts: true (see packages/enclave-config/tsup.config.js). Building ESM and CJS in parallel with dts enabled can cause duplicate generation or overwrites. Recommend enabling DTS for one build only.
Apply this diff to disable DTS in the CJS build:
{ ...baseConfig, include: ["./src/**/*.ts"], format: ["cjs"], outExtension: () => ({ js: ".cjs", }), + dts: false, esbuildOptions: (options) => { options.alias = { "@gnosis-guild/e3-wasm/init": "../../crates/wasm/init_node.cjs", }; }, },examples/CRISP/apps/client/src/utils/proof-encoding.ts (1)
35-49: Tighten types and validate noirPublicInputs as 32-byte hexnoirPublicInputs are assumed to be 0x-prefixed 32-byte values but currently typed as string[] and cast. This risks runtime failures if inputs lack 0x or are the wrong length. Viem’s ABI encoder expects proper Hex strings.
Two small improvements:
- Type the inputs/return as Hex to catch issues at compile time.
- Optionally validate at runtime to fail early with a clear error.
Apply this diff:
-import { hexToBytes, encodeAbiParameters, parseAbiParameters, bytesToHex } from 'viem'; +import { hexToBytes, encodeAbiParameters, parseAbiParameters, bytesToHex, isHex } from 'viem'; +import type { Hex } from 'viem'; const crispAbi = parseAbiParameters( '(bytes, bytes, bytes32[], bytes)' ); export const encodeCrispInputs = ( semaphoreProof: Uint8Array, noirProof: Uint8Array, - noirPublicInputs: string[], + noirPublicInputs: Hex[], encryptedVote: Uint8Array -): string => { - return encodeAbiParameters(crispAbi, [ +): Hex => { + // Optional runtime guard to ensure 32-byte hex inputs + for (const input of noirPublicInputs) { + if (!isHex(input, { strict: true }) || (input.length !== 66 /* 0x + 64 hex */)) { + throw new Error(`Invalid noirPublicInput: expected 0x-prefixed 32-byte hex, got "${input}"`); + } + } + return encodeAbiParameters(crispAbi, [ [ bytesToHex(semaphoreProof), bytesToHex(noirProof), - noirPublicInputs.map(input => input as `0x${string}`), + noirPublicInputs, bytesToHex(encryptedVote) ] ]) }If noirPublicInputs are not guaranteed to be hex, consider accepting Uint8Array[] instead and converting via bytesToHex here.
examples/CRISP/hardhat.config.ts (2)
84-92: Confirm private key shape for localhost/hardhat accounts
- hardhat: using the array of { privateKey, balance } objects is valid. The balance here is extremely large; ensure this won’t overflow any tooling parsing BN-like values. A more typical dev balance (e.g., "1000000000000000000000000") is sufficient.
- localhost: "accounts: [privateKey]" expects 0x-prefixed keys; the normalization above guards this.
If you want to soften the balance and keep readability:
- accounts: [{privateKey, balance: "1000000000000000000000000000000000000000"}], + accounts: [{ privateKey, balance: "1000000000000000000000000" }], // 1e24 wei
36-47: Optional: Support RPC_URL fallback for non-Infura chainsCurrently, non-Infura chains have hardcoded URLs, but the default branch for others mandates an INFURA_API_KEY. Allowing RPC_URL as a fallback improves DX in environments without Infura keys.
Example:
switch (chain) { case "avalanche": jsonRpcUrl = "https://api.avax.network/ext/bc/C/rpc"; break; case "bsc": jsonRpcUrl = "https://bsc-dataseed1.binance.org"; break; default: - jsonRpcUrl = "https://" + chain + ".infura.io/v3/" + infuraApiKey; + jsonRpcUrl = process.env.RPC_URL || ("https://" + chain + ".infura.io/v3/" + infuraApiKey); }examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (1)
16-27: Hardcoded contract addresses and comment mismatchThe comment states defaults “do not matter”, yet hardcoded contracts and chain settings are present. If these are not used for this worker’s local encryption path, prefer omitting or parameterizing them to avoid confusion and accidental mainnet/testnet misconfig.
Consider passing these from the main thread via
postMessagepayload or environment config, or omit them if SDK.create doesn’t require them for local-only ops.packages/enclave-sdk/package.json (3)
9-15: Dual ESM/CJS entry points look good; consider relying solely on exportsThe import/require mapping plus main/module/types fields are consistent. To reduce ambiguity for Node’s resolver, consider relying solely on the exports map (keeping main/module for legacy tooling if needed).
No action required if your consumers/build matrix is already green.
37-38: Verify Noir/bb.js versions and bundle size impactAdding @aztec/bb.js and @noir-lang/noir_js is significant. Ensure:
- Versions are compatible with your Greco/Noir integration.
- Tree-shaking is effective (consider
"sideEffects": falseif applicable) to keep SDK size reasonable.I can provide a bundle-size check script if helpful.
Optionally add:
"types": "./dist/index.d.ts", + "sideEffects": false,
20-21: Prebuild script cross-package navigationThe prebuild script cd’s into ../evm and ../../crates/wasm. This can be brittle in CI if run from different CWDs.
Consider using workspace scripts (pnpm -w run …) or
turbo/nxtask pipelines with explicit deps for reproducibility.examples/CRISP/package.json (1)
35-36: Mark example package as private to prevent accidental publishExamples should generally not be published.
Consider:
{ "name": "crisp", + "private": true, "version": "0.0.0",crates/wasm/package.json (1)
6-8: ESM/CJS entry points defined; verify consumer compatibilityDefining main (nodejs CJS), module (web ESM), and types (web) is fine. Ensure that Node consumers that use ESM import (import …) resolve to the correct build, and browser bundlers pick the web build.
If you support both, consider adding explicit import/require conditions under "." similar to "./init" to remove ambiguity:
"exports": { ".": { - "node": { + "node": { "types": "./dist/nodejs/e3_wasm.d.ts", "default": "./dist/nodejs/e3_wasm.js" }, "browser": { "types": "./dist/web/e3_wasm.d.ts", "default": "./dist/web/e3_wasm.js" }, - "default": { - "types": "./dist/web/e3_wasm.d.ts", - "default": "./dist/web/e3_wasm.js" - } + "import": "./dist/web/e3_wasm.js", + "require": "./dist/nodejs/e3_wasm.js", + "default": "./dist/web/e3_wasm.js" },This makes CJS vs ESM intent explicit for non-conditional environments.
examples/CRISP/test/crisp.contracts.test.ts (2)
121-121: Avoid async describe callbacks in Mocha.
describeshould be synchronous; usingasynchere can lead to flaky test discovery.Apply this diff:
-describe("voting", async () => { +describe("voting", () => {
164-165: Decode committee public key to bytes if the server returns hex.The server’s
committee_public_keyis likely hex-encoded in JSON. Ensure it’s aUint8Arraybefore passing to the SDK.Apply this diff:
- const publicKey = stateJson.committee_public_key + const publicKey = + typeof stateJson.committee_public_key === "string" + ? hexToBytes(stateJson.committee_public_key as `0x${string}`) + : stateJson.committee_public_keyIf the API already returns a byte array, this remains a no-op.
packages/enclave-sdk/src/greco.ts (2)
128-132: Consider caching Noir/Backend instances to avoid repeated heavy initialization.Constructing
NoirandUltraHonkBackendper call can be expensive. If the circuit is reused, cache these per-circuit to improve throughput.For example, maintain a simple in-memory map keyed by circuit hash/bytecode to reuse initialized backends.
146-176: Optional: pre-validate input shapes to fail fast.Greco expects specific sizes for each polynomial array (e.g., matching L/size). Adding guards to check lengths before
noir.executewill yield clearer error messages.crates/bfv-helpers/src/client.rs (5)
35-41: Nit: Renamedatatovotefor consistency with docs/tests.Both the docs and the verifiable variant use
vote. Keep naming consistent.Apply this diff:
-pub fn bfv_encrypt_u64( - data: u64, +pub fn bfv_encrypt_u64( + vote: u64, public_key: Vec<u8>, degree: usize, plaintext_modulus: u64, moduli: [u64; 1], ) -> Result<Vec<u8>> { @@ - let input = vec![data]; + let input = vec![vote];
59-62: Consider serde derives for cross-boundary serialization.If this struct is passed across FFI/WASM boundaries, deriving Serialize/Deserialize will simplify bridging. Ignore if not required.
Apply this diff:
-#[derive(Debug, Clone)] +#[derive(Debug, Clone)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct VerifiableEncryptionResult { pub encrypted_vote: Vec<u8>, pub circuit_inputs: String, }And add
serdeas an optional feature if desired.
116-121: Avoid unwrap on constant modulus parse; propagate errors.Even though this is a constant, prefer not to
unwrap()inside library code.Apply this diff:
- let zkp_modulus = BigInt::from_str_radix( - "21888242871839275222246405745257275088548364400416034343698204186575808495617", - 10, - ) - .unwrap(); + let zkp_modulus = BigInt::from_str_radix( + "21888242871839275222246405745257275088548364400416034343698204186575808495617", + 10, + ).map_err(|e| anyhow!("Invalid BN254 modulus: {}", e))?;
41-41: Upstream panics from parameter builder.
build_bfv_params_arcpanics on invalid inputs. Since this function returnsResult, consider returning an error instead of panicking when params are invalid. This will make the API safer for caller-provided parameters.If acceptable, expose a
try_build_bfv_params_arc(...) -> anyhow::Result<Arc<BfvParameters>>and use it here.
84-89: API generality: consider accepting a slice for moduli.You currently restrict to
[u64; 1]. If multi-modulus sets are on the roadmap, prefer&[u64](orimpl AsRef<[u64]>) for future-proofing.crates/wasm/Cargo.toml (1)
15-15: Nice dependency trim; consider unifyingwasm-bindgenvia workspace for version consistencyDropping direct
rand/fhe_*deps here is good. Minor nit: if your workspace already manages versions for shared crates, consider movingwasm-bindgen = "0.2.99"under[workspace.dependencies]to avoid version skew across crates.packages/enclave-sdk/src/types.ts (2)
270-275: FheProtocol enum scoped to BFV onlyScoping to BFV is fine for now and keeps the surface area tight. Consider documenting “experimental/preview” status if other schemes are expected soon.
284-297: ProtocolParams: consider naming clarity for single modulus
moduliis a singlebigint. The example/comment shows[0x3FFFFFFF000001](array of one), which may cause confusion. If we truly support a single modulus at the moment, consider naming itmodulus(singular), or adding a brief comment clarifying that BFV currently uses a single modulus.-export interface ProtocolParams { +export interface ProtocolParams { degree: number; plaintextModulus: bigint; - moduli: bigint; + /** Single BFV modulus (currently one prime). Named plural to align with future extensions. */ + moduli: bigint; }crates/wasm/src/lib.rs (1)
11-21: Docstring mentions “default params” but function accepts explicit paramsThe doc claims “default params” while the function requires explicit
degree,plaintext_modulus, andmoduli. Update the comment for accuracy.-/// A function to encrypt a u64 value using BFV and default params. +/// Encrypt a u64 value using BFV with explicit parameters.packages/enclave-sdk/tests/sdk.test.ts (3)
20-31: SDK factory usage looks good, but rpcUrl is emptyInstantiating viem clients with
rpcUrl: ""can be brittle in some environments. It works as long as you don’t perform RPC calls, but consider using a dummy HTTP URL in tests to avoid transport construction edge cases.- rpcUrl: "", + rpcUrl: "http://localhost:8545",
32-40: Typos in test description“environent” -> “environment”.
- it("should encrypt a number without crashing in a node environent", async () => { + it("should encrypt a number without crashing in a node environment", async () => {
41-51: Proof test: assertion style and timeout
- Typo in description (“environent”).
- Prefer a reasonable timeout (e.g., 60s) unless you expect very heavy work.
- For Vitest,
toBeInstanceOfis clearer; or in Chai style,to.be.instanceof.- it("should encrypt a number and generate a proof without crashing in a node environent", async () => { + it("should encrypt a number and generate a proof without crashing in a node environment", async () => { @@ - expect(value).to.be.an.instanceof(Object); - expect(value.encryptedVote).to.be.an.instanceof(Uint8Array); - expect(value.proof).to.be.an.instanceOf(Object) - }, 9999999); + expect(value).toBeInstanceOf(Object); + expect(value.encryptedVote).toBeInstanceOf(Uint8Array); + expect(value.proof).toBeInstanceOf(Object); + }, 60_000);examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts (1)
73-81: Avoid sending plaintext vote in proof messageYou’re already noting this in the TODO. The code still sets
message = String(pollSelected.value), which leaks the vote in the public inputs. Consider hashing the vote with a domain separator and salt before including it in the Semaphore proof.Example refinement:
- // @todo let's not send the vote in plaintext - const message = String(pollSelected.value); + // Avoid leaking the vote: hash with domain + salt + const domain = BigInt(roundState.id); + const salt = BigInt(Date.now()); // or a deterministic/user-specific salt + const message = (domain << 128n) ^ (BigInt(pollSelected.value) << 64n) ^ salt;If you’d like, I can wire this to a Poseidon/keccak hash depending on your verification circuit assumptions.
packages/enclave-sdk/src/enclave-sdk.ts (2)
97-110: initialize(): consider removal or making it internalYou’ve marked this as redundant. Given most calls gate on
this.initialized, decide whether to:
- Remove it and inline initialization where needed, or
- Keep it but make it private and actually initialize wasm/clients as needed.
171-201: requestE3 now accepts gasLimit; consider documenting EVM gas behaviorSmall nit: you added a
gasLimitplumb-through. Consider documenting in README/API docs how this interacts with chain defaults and estimation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (5)
Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasmis excluded by!**/*.wasmexamples/CRISP/apps/server/Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/wasm-crypto/Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (46)
crates/bfv-helpers/Cargo.toml(1 hunks)crates/bfv-helpers/src/client.rs(2 hunks)crates/bfv-helpers/src/lib.rs(1 hunks)crates/bfv-helpers/src/util.rs(0 hunks)crates/evm-helpers/src/contracts.rs(1 hunks)crates/wasm/Cargo.toml(1 hunks)crates/wasm/README.md(1 hunks)crates/wasm/init_node.cjs(1 hunks)crates/wasm/init_node.js(1 hunks)crates/wasm/init_web.js(1 hunks)crates/wasm/package.json(2 hunks)crates/wasm/src/lib.rs(1 hunks)examples/CRISP/.env.example(1 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.js(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js(1 hunks)examples/CRISP/apps/client/package.json(1 hunks)examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts(3 hunks)examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx(1 hunks)examples/CRISP/apps/client/src/model/vote.model.ts(1 hunks)examples/CRISP/apps/client/src/utils/proof-encoding.ts(1 hunks)examples/CRISP/apps/client/src/utils/proofUtils.ts(0 hunks)examples/CRISP/apps/server/src/server/mod.rs(1 hunks)examples/CRISP/apps/server/src/server/models.rs(1 hunks)examples/CRISP/apps/server/src/server/routes/voting.rs(3 hunks)examples/CRISP/apps/wasm-crypto/Cargo.toml(0 hunks)examples/CRISP/apps/wasm-crypto/README.md(0 hunks)examples/CRISP/apps/wasm-crypto/src/lib.rs(0 hunks)examples/CRISP/contracts/CRISPVerifier.sol(12 hunks)examples/CRISP/hardhat.config.ts(2 hunks)examples/CRISP/package.json(3 hunks)examples/CRISP/test/crisp.contracts.test.ts(1 hunks)examples/CRISP/tsconfig.json(1 hunks)packages/enclave-react/src/useEnclaveSDK.ts(4 hunks)packages/enclave-sdk/package.json(2 hunks)packages/enclave-sdk/src/enclave-sdk.ts(6 hunks)packages/enclave-sdk/src/greco.ts(1 hunks)packages/enclave-sdk/src/index.ts(2 hunks)packages/enclave-sdk/src/types.ts(2 hunks)packages/enclave-sdk/src/wasm.ts(0 hunks)packages/enclave-sdk/tests/sdk.test.ts(1 hunks)packages/enclave-sdk/tsup.config.js(1 hunks)packages/evm/package.json(1 hunks)templates/default/client/src/pages/WizardSDK.tsx(2 hunks)templates/default/tests/integration.spec.ts(3 hunks)
💤 Files with no reviewable changes (9)
- crates/bfv-helpers/src/util.rs
- packages/enclave-sdk/src/wasm.ts
- examples/CRISP/apps/wasm-crypto/Cargo.toml
- examples/CRISP/apps/wasm-crypto/README.md
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts
- examples/CRISP/apps/client/src/utils/proofUtils.ts
- examples/CRISP/apps/wasm-crypto/src/lib.rs
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.js
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
PR: gnosisguild/enclave#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:
crates/bfv-helpers/src/lib.rs
🧬 Code Graph Analysis (15)
crates/wasm/init_node.cjs (2)
crates/wasm/init_node.js (1)
initializeWasm(7-9)crates/wasm/init_web.js (1)
initializeWasm(8-10)
examples/CRISP/apps/server/src/server/mod.rs (1)
examples/CRISP/apps/server/src/logger.rs (1)
init_logger(13-33)
crates/wasm/init_web.js (1)
crates/wasm/init_node.js (1)
initializeWasm(7-9)
crates/wasm/init_node.js (1)
crates/wasm/init_web.js (1)
initializeWasm(8-10)
packages/enclave-sdk/src/greco.ts (1)
packages/enclave-sdk/src/index.ts (1)
generateProof(67-67)
packages/enclave-sdk/tsup.config.js (1)
packages/enclave-config/tsup.config.js (2)
baseConfig(9-16)baseConfig(9-16)
examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (2)
packages/enclave-sdk/src/index.ts (2)
EnclaveSDK(8-8)FheProtocol(42-42)packages/enclave-sdk/src/enclave-sdk.ts (1)
EnclaveSDK(42-474)
crates/bfv-helpers/src/client.rs (5)
crates/bfv-helpers/src/lib.rs (1)
build_bfv_params_arc(76-90)packages/enclave-sdk/src/index.ts (1)
VerifiableEncryptionResult(38-38)packages/enclave-sdk/src/types.ts (1)
VerifiableEncryptionResult(256-265)crates/fhe/src/fhe.rs (1)
from_bytes(169-171)templates/default/program/src/lib.rs (1)
test(37-72)
crates/wasm/src/lib.rs (1)
crates/bfv-helpers/src/client.rs (2)
bfv_encrypt_u64(34-56)bfv_verifiable_encrypt_u64(83-128)
templates/default/client/src/pages/WizardSDK.tsx (2)
packages/enclave-sdk/src/enclave-sdk.ts (1)
publishInput(221-231)packages/enclave-sdk/src/contract-client.ts (1)
publishInput(183-223)
templates/default/tests/integration.spec.ts (3)
examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (1)
sdk(18-28)packages/enclave-sdk/src/index.ts (1)
EnclaveEventType(42-42)packages/enclave-react/src/index.ts (1)
EnclaveEventType(32-32)
packages/enclave-react/src/useEnclaveSDK.ts (2)
packages/enclave-sdk/src/index.ts (2)
FheProtocol(42-42)ProtocolParams(37-37)packages/enclave-sdk/src/types.ts (1)
ProtocolParams(284-297)
packages/enclave-sdk/tests/sdk.test.ts (3)
examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (1)
sdk(18-28)packages/enclave-sdk/src/index.ts (2)
EnclaveSDK(8-8)FheProtocol(42-42)packages/enclave-sdk/src/enclave-sdk.ts (1)
EnclaveSDK(42-474)
examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts (3)
examples/CRISP/apps/client/src/utils/proof-encoding.ts (2)
encodeSemaphoreProof(14-29)encodeCrispInputs(35-49)examples/CRISP/test/crisp.contracts.test.ts (1)
encodeSemaphoreProof(47-62)examples/CRISP/apps/client/src/model/vote.model.ts (1)
BroadcastVoteRequest(24-28)
packages/enclave-sdk/src/enclave-sdk.ts (6)
packages/enclave-sdk/src/index.ts (5)
FheProtocol(42-42)ProtocolParams(37-37)BfvProtocolParams(42-42)VerifiableEncryptionResult(38-38)generateProof(67-67)packages/enclave-sdk/src/types.ts (3)
ProtocolParams(284-297)BfvProtocolParams(302-314)VerifiableEncryptionResult(256-265)crates/wasm/init_node.js (1)
initializeWasm(7-9)crates/wasm/init_web.js (1)
initializeWasm(8-10)crates/wasm/src/lib.rs (2)
bfv_encrypt_number(28-38)bfv_verifiable_encrypt_number(59-74)packages/enclave-sdk/src/greco.ts (2)
CircuitInputs(23-37)generateProof(128-179)
🪛 dotenv-linter (3.3.0)
examples/CRISP/.env.example
[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [UnorderedKey] The MNEMONIC key should go before the PRIVATE_KEY key
(UnorderedKey)
[warning] 3-3: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 Gitleaks (8.27.2)
examples/CRISP/test/crisp.contracts.test.ts
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Biome (2.1.2)
packages/enclave-sdk/src/enclave-sdk.ts
[error] 149-155: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 157-157: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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_e3_support_dev
- GitHub Check: Build & Push Image
- GitHub Check: build_enclave_cli
- GitHub Check: build_sdk
- GitHub Check: test_contracts
- GitHub Check: integration_prebuild
- GitHub Check: test_net
- GitHub Check: Build & Push Image
- GitHub Check: build_e3_support_risc0
- GitHub Check: rust_unit
- GitHub Check: e2e
🔇 Additional comments (50)
examples/CRISP/tsconfig.json (1)
10-11: Keeping resolveJsonModule enabled is appropriateGood call retaining resolveJsonModule; it’s frequently useful for task/deploy/test setups that read JSON artifacts.
examples/CRISP/contracts/CRISPVerifier.sol (7)
7-7: Public inputs increased to 8223 — consistent with VK initializationNUMBER_OF_PUBLIC_INPUTS and vk.publicInputsSize are aligned at 8223. This is necessary for the transcript and delta computations below. Looks good.
Also applies to: 13-13
272-272: Introduce PAIRING_POINTS_SIZE = 16Clear and improves readability by making the pairing-point object size explicit across the codebase.
389-392: Proof struct extended with pairingPointObjectAdding Fr[16] pairingPointObject to Honk.Proof is coherent with the rest of the changes (transcript, delta, and proof loader). No concerns.
1563-1564: PROOF_SIZE = 456 — matches the new loader layoutManual check: 16 Fr (512 B) + 3 G1ProofPoints (384 B) + 5 G1ProofPoints (640 B) + 224 Fr (7168 B) + 40 Fr (1280 B) + 27 G1ProofPoints (3456 B) + 28 Fr (896 B) + 2 G1ProofPoints (256 B) = 14592 B = 456 * 32. Correct.
1586-1588: computePublicInputDelta now includes pairingPointObjectPassing
p.pairingPointObjectinto the delta computation keeps the verifier’s beta/gamma folding consistent with the transcript that also includes the pairing points. Good.
1600-1635: Consider optional gas micro-optimizations incomputePublicInputDeltaAll logic and ordering look correct, but you can shave off a few gas units by:
- Caching the loop bound
- Using
unchecked { ++i; }instead ofi++- Reusing a pre-wrapped constant (
Fr one = Fr.wrap(1);) if you’re constructing it elsewhereExample diff:
{ - for (uint256 i = 0; i < numPublicInputs - PAIRING_POINTS_SIZE; i++) { + uint256 mainLen = numPublicInputs - PAIRING_POINTS_SIZE; + for (uint256 i; i < mainLen; ) { Fr pubInput = FrLib.fromBytes32(publicInputs[i]); numerator = numerator * (numeratorAcc + pubInput); denominator = denominator * (denominatorAcc + pubInput); numeratorAcc = numeratorAcc + beta; denominatorAcc = denominatorAcc - beta; + unchecked { ++i; } } - for (uint256 i = 0; i < PAIRING_POINTS_SIZE; i++) { + for (uint256 i; i < PAIRING_POINTS_SIZE; ) { Fr pubInput = pairingPointObject[i]; numerator = numerator * (numeratorAcc + pubInput); denominator = denominator * (denominatorAcc + pubInput); numeratorAcc = numeratorAcc + beta; denominatorAcc = denominatorAcc - beta; + unchecked { ++i; } } }Please manually profile the gas impact on your target networks (for example, with Foundry’s
forge test --gas-report) to ensure these tweaks are worth it.
1576-1579: Verify client is supplying 8207 public inputs (EXCLUDING pairing points)
The on-chain guard in CRISPVerifier.sol now requirespublicInputs.length == vk.publicInputsSize - PAIRING_POINTS_SIZE // i.e. 8223 - 16 = 8207but on the client side:
useWebAssemblyHook(examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx) does
const { proof, publicInputs } = proofDataencodeCrispInputs(examples/CRISP/apps/client/src/utils/proof-encoding.ts) forwardsnoirPublicInputsunchangedWe didn’t find any trimming of the 16 pairing points in the repo. Please confirm (or update) that:
- The WebAssembly worker / SDK now returns exactly 8207 entries in
proofData.publicInputs(i.e. has stripped out thepairingPointObjectelements) before encoding.- All client-side calls to
encodeCrispInputs(…, publicInputs, …)actually receive an array of length 8207.examples/CRISP/apps/server/src/server/mod.rs (1)
26-26: Import re-ordering is fine.Moving the logger import has no functional impact. start() still initializes the logger before any logging is used.
examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (3)
36-43: Message payload shape: confirm downstream expectationsYou now send
encryptedVote: { vote, proofData }. Downstream code used to accept different shapes (previously raw bytes or different field names). Ensure all consumers are updated accordingly (e.g., proof encoding and crisp_inputs generation).Would you like a quick grep script to verify all consumers of
encrypt_voteworker messages handle the new shape?
18-29: Await async SDK.create, use valid chain key and non-empty RPC/KeyIn
examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js(lines 18–29), the SDK is initialized without awaiting the async factory, with a numericchainId, and empty RPC/Key values:• EnclaveSDK.create returns a Promise—add
awaitto avoid mis-calling methods on the promise.
•chainIdmust be one of the keys inEnclaveSDK.chains(e.g.'anvil'/'localhost'), not the raw number.
• Passing""forrpcUrl/privateKeyrisks misconfiguration if the SDK eagerly instantiates a provider.Suggested diff (adjust to your environment):
- const sdk = EnclaveSDK.create({ + const sdk = await EnclaveSDK.create({ // use a valid chain key exposed by EnclaveSDK.chains - chainId: 31337, + chainId: 'anvil', contracts: { enclave: "0xc6e7DF5E7b4f2A278906862b61205850344D4e7d", ciphernodeRegistry: "0xc6e7DF5E7b4f2A278906862b61205850344D4e7d", }, - rpcUrl: "", - privateKey: "", + rpcUrl: "http://127.0.0.1:8545", + // omit or supply a real privateKey only if needed + // privateKey: undefined, protocol: FheProtocol.BFV, });Optional: cache the SDK at module scope to avoid re-initializing:
let sdkPromise; async function getSdk() { if (!sdkPromise) { sdkPromise = EnclaveSDK.create({ chainId: 'anvil', contracts: { enclave: "...", ciphernodeRegistry: "..." }, rpcUrl: "http://127.0.0.1:8545", protocol: FheProtocol.BFV, }); } return sdkPromise; } // inside handler const sdk = await getSdk();Please confirm:
- Does
EnclaveSDK.createreturn a Promise?- Which string keys does
EnclaveSDK.chainsexpose for your network?- Are
rpcUrlandprivateKeyoptional in your setup, or must they be non-empty here?
30-34: Ensure correct types forvoteIdandpublicKeybefore callingencryptNumberAndGenProofThe EnclaveSDK binding for
encryptNumberAndGenProofrequires:
voteIdas a bigint to avoid JS number precision loss.publicKeyas a Uint8Array (rather than a hex string).Consider adding explicit guards and conversions:
- const result = await sdk.encryptNumberAndGenProof( - voteId, + const result = await sdk.encryptNumberAndGenProof( + typeof voteId === 'bigint' ? voteId : BigInt(voteId), + typeof publicKey === 'string' ? hexToBytes(publicKey) : publicKey, circuit );• Confirm the exact parameter signatures in
@gnosis-guild/enclave-sdkdocumentation.
• Ensure any hex‐encoded public key from the UI is converted (e.g. viahexToBytes).
• IfvoteIdmay already be a bigint, guard the cast to avoid errors.packages/evm/package.json (1)
136-137: No circular dependency detected
The change adds@gnosis-guild/enclave-sdkto packages/evm/package.json, not to the@gnosis-guild/enclavepackage. While:
packages/enclave-sdkdepends on@gnosis-guild/enclavepackages/evmdepends on@gnosis-guild/enclave-sdkThere is no dependency path from
@gnosis-guild/enclaveback topackages/evm, so no cycle is introduced. Please disregard the circular-dependency warning.Likely an incorrect or invalid review comment.
examples/CRISP/package.json (3)
20-21: New test:contracts script LGTMThe addition aligns with the new CRISP contract tests.
27-31: Runtime deps update looks consistentAdding @gnosis-guild/enclave-sdk and ethers is consistent with the new flow.
46-48: GitHub semver reference for semaphore core and matching data versionUsing a GitHub ref for @semaphore-protocol/core with a dist tag is fine short-term. Ensure it matches @semaphore-protocol/data and the Noir circuits used in tests.
I can add a small script to list resolved versions in the lockfile to ensure consistency.
crates/wasm/package.json (1)
34-47: Good split of init entry points for node/web; ensure typings match the renameYou mapped "./init" to init_node.js/init_node.cjs and init_web.js and updated the conditions. Ensure init.d.ts exports the correct symbol (initializeWasm) matching the implementation rename.
If init.d.ts still uses a default export named differently, TypeScript consumers will break. I can suggest a quick patch if needed.
examples/CRISP/test/crisp.contracts.test.ts (2)
69-73: Confirm the proof generator uses the same RPC URL as the SDK.
SemaphoreEthersusesrpcUrlfrom env, while the SDK previously used a hardcoded URL. After adopting env for the SDK, ensureprocess.env.RPC_URLpoints to the same local Hardhat node to avoid mismatched chain state.Would you like me to add a fallback to
http://localhost:8545here as well ifRPC_URLis not set?
175-183: VerifyProofDatafields against test encodingIn examples/CRISP/test/crisp.contracts.test.ts (lines 172-183), the test does:
const { proof, encryptedVote } = await sdk.encryptNumberAndGenProof(...) const encodedInputs = AbiCoder.defaultAbiCoder().encode( ["bytes", "bytes", "bytes32[]", "bytes"], [semaphoreProof, proof.proof, proof.publicInputs, encryptedVote] )–
encryptNumberAndGenProofreturnsproof: ProofData(from@aztec/bb.js).
– Confirm thatProofDataactually exposes:
•proofas aUint8Array(ABIbytes)
•publicInputsas an array that ABI-encodes tobytes32[]If the external
ProofDatainterface differs, either:
• Map or rename its fields before encoding
• Adjust the test to match the actual shapepackages/enclave-sdk/src/greco.ts (1)
77-97: Default Greco params look coherent for BN254-backed circuits.The constants align with a single-modulus BFV setup and BN254 field usage. Good to have them centralized.
crates/bfv-helpers/src/lib.rs (1)
8-8: Module wiring looks good.
mod util;inclusion is fine; no public API change. The parameter utilities and ABI encode/decode helpers are cohesive and well-tested.examples/CRISP/apps/client/package.json (1)
20-20: Workspace linkage to enclave-sdk looks appropriate.Using
workspace:*for the example app is fine within the monorepo. No issues.packages/enclave-react/src/useEnclaveSDK.ts (2)
14-16: Type imports look correctImporting
FheProtocolandProtocolParamsas type-only avoids bundling value exports unnecessarily. No issues here.
112-122: WiringprotocolandprotocolParamsinto SDKConfig looks goodForwarding the new fields into
EnclaveSDKis correct.examples/CRISP/apps/client/src/model/vote.model.ts (2)
24-28: LGTM: broadcast payload simplified to a singlecrisp_inputsfieldThis aligns well with the server model/routing changes and the on-chain
publish_inputusage.
54-59: Ignore obsolete-interface suggestion –EncryptedVoteis still in active useThe
EncryptedVoteTS interface is used by:
useWebAssemblyhook (returnsPromise<EncryptedVote>)VoteManagementcontext types (encryptVote,broadcastVotesignatures)It shouldn’t be removed or reshaped to a single
crisp_inputsfield. If you need to translate this structure into the server’sEncryptedVote { crisp_inputs: String… }, do so when constructing the outbound request (e.g. in your broadcast wrapper), not by changing the returned WebAssembly type.Likely an incorrect or invalid review comment.
examples/CRISP/apps/server/src/server/models.rs (1)
53-53: LGTM! Simplified payload structure.The replacement of multiple separate fields (
enc_vote_bytes,proof,public_inputs,proof_sem) with a singlecrisp_inputsstring field simplifies the data model and aligns with the consolidated proof encoding approach described in the PR objectives.templates/default/client/src/pages/WizardSDK.tsx (4)
26-26: Updated import for new API.The import correctly switches from the old
encryptNumberto the newbfvVerifiableEncryptNumberfunction, aligning with the verifiable encryption changes.
789-790: Proper usage of new verifiable encryption API.The code correctly uses
bfvVerifiableEncryptNumberand properly handles the structured return value with theencryptedVoteproperty.
797-797: Correct property access for encrypted data.The code properly accesses
encryptedInput1.encryptedVoteto get the ciphertext bytes for publishing, which aligns with the newVerifiableEncryptionResultstructure.
802-802: Consistent usage pattern.The second input follows the same pattern as the first, accessing
encryptedInput2.encryptedVotefor the ciphertext bytes.examples/CRISP/apps/server/src/server/routes/voting.rs (3)
15-15: Updated imports for simplified payload handling.The import correctly adds
Bytesfrom alloy primitives to handle the hex-decoded input data, removing the need for complex DynSolValue encoding.
95-95: Simplified blockchain interaction.The direct usage of
publish_inputwithinput_bytes_alloysimplifies the contract interaction by removing the complex DynSolValue encoding, which aligns with the PR's goal to simplify the SDK experience.
113-131: Well-implemented error handling and rollback.The
handle_vote_errorfunction properly handles blockchain failures by rolling back the voter's address in storage and returning an appropriate error response.packages/enclave-sdk/src/index.ts (3)
37-39: Expanded type exports for verifiable encryption.The addition of
ProtocolParamsandVerifiableEncryptionResultto the public API properly exposes the new BFV verifiable encryption types to consumers.
41-42: Enhanced protocol and parameter exports.The addition of
FheProtocolandBfvProtocolParamsto the exported enums/constants provides necessary protocol configuration options for consumers.
67-67: Updated utility export for proof generation.The replacement of
encryptNumberwithgenerateProoffrom the greco module aligns with the shift towards verifiable encryption and proof generation workflows.templates/default/tests/integration.spec.ts (3)
237-238: Updated to use SDK instance method.The encryption calls correctly use the SDK instance method
sdk.encryptNumber()instead of the previous top-level export, which aligns with the new SDK architecture.
243-249: Simplified hex encoding.The hex encoding for publishing inputs has been simplified with a more concise
join("")operation, improving readability.
19-19: Missing import for EProtocol.The test references
EProtocol.BFVon line 170 but doesn't importEProtocol. This should be imported from the SDK.import { AllEventTypes, calculateStartWindow, DEFAULT_COMPUTE_PROVIDER_PARAMS, DEFAULT_E3_CONFIG, E3, EnclaveEvent, EnclaveEventType, EnclaveSDK, encodeBfvParams, encodeComputeProviderParams, RegistryEventType, - EProtocol, + FheProtocol, } from "@gnosis-guild/enclave-sdk";Likely an incorrect or invalid review comment.
packages/enclave-sdk/src/types.ts (4)
7-13: Type-only imports and ProofData wiring look correctUsing type-only imports from viem and bb.js keeps bundling lean. The
ProofDataimport matches downstream usage inVerifiableEncryptionResult.
19-58: SDKConfig: protocol and protocolParams addition is soundThe new
protocol: FheProtocoland optionalprotocolParams?: ProtocolParamsfields are well-integrated. This enables protocol-driven configuration without breaking existing consumers.
256-265: VerifiableEncryptionResult shape aligns with SDK usage
{ encryptedVote: Uint8Array; proof: ProofData }matches the return value built inencryptNumberAndGenProof. No issues here.
302-314: BFV normal params: defaults look reasonable and typed correctlyGood use of
as const satisfies ProtocolParams. The values line up with the rest of the stack.examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts (2)
82-88: Shape of voteEncrypted expected by encodeCrispInputs
encodeCrispInputsexpects(semaphoreProof: Uint8Array, noirProof: Uint8Array, noirPublicInputs: string[], encryptedVote: Uint8Array). EnsurevoteEncryptedconforms:proof: Uint8Array,public_inputs: string[], andvote: Uint8Array. IfencryptVotewas refactored to return{ encryptedVote, proof }, this call will break.Would you like me to generate a quick verification script to scan usages/definitions of
encryptVoteand its return type across the repo?
112-117: UX: broadcast failure feedback is a good additionNice user-facing toast for the
failed_broadcastbranch.packages/enclave-sdk/src/enclave-sdk.ts (4)
27-38: Type imports and greco/wasm wiring are consistentThe glue between
initializeWasm, wasm exports, and Greco proof helpers is sound. Types line up (CompiledCircuit,VerifiableEncryptionResult,ProtocolParams).
79-91: Protocol defaulting is correct; error on unsupported protocolsGood fallback to
BfvProtocolParams.BFV_NORMALwhenprotocolParamsare not supplied, and explicit error on unsupported protocols.
118-132: encryptNumber: protocol switch is fineReturning the wasm result directly after
await initializeWasm()is appropriate. Keep in mind wasm-bindgen’s u64 expects BigInt in JS; your types already reflect that.
430-473: Factory: includes protocol configuration; good separation of concernsThe static factory handles transport selection and wallet client wiring cleanly. Passing protocol config to the constructor keeps responsibilities clear.
ctrlc03
force-pushed
the
feat/wasm-crypto-to-sdk
branch
from
61d1370 to
0aec1b4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
packages/enclave-sdk/package.json (1)
36-47: Move build‐only plugins and test runner to devDependenciesI’ve confirmed that:
vite-plugin-wasmandvite-plugin-top-level-awaitare only referenced in README/examples (i.e., build‐time only).vitestis only imported inpackages/enclave-sdk/tests/*.ts(test‐time only).Please update
packages/enclave-sdk/package.jsonaccordingly:"devDependencies": { "@gnosis-guild/enclave-config": "workspace:*", "concurrently": "^9.1.2", "tsup": "^8.5.0", "typescript": "5.8.3", "vite": "^6.2.0", - "vite-plugin-dts": "^4.5.3" + "vite-plugin-dts": "^4.5.3", + "vite-plugin-top-level-await": "^1.5.0", + "vite-plugin-wasm": "^3.4.1", + "vitest": "^1.6.1" }, "dependencies": { "@aztec/bb.js": "^0.82.2", "@noir-lang/noir_js": "1.0.0-beta.3", "@gnosis-guild/e3-wasm": "workspace:*", "@gnosis-guild/enclave": "workspace:*", "comlink": "^4.4.2", "viem": "2.30.6", - "vite-plugin-top-level-await": "^1.5.0", - "vite-plugin-wasm": "^3.4.1", - "vitest": "^1.6.1", "web-worker": "^1.5.0" }
- Remove the three packages from
dependencies.- Add them under
devDependenciesto avoid bloating consumer installs.
♻️ Duplicate comments (9)
packages/enclave-sdk/src/greco.ts (1)
131-132: Remove unsupported UltraHonkBackendthreadsoption (not honored here).UltraHonkBackend doesn’t accept a top-level
threadsoption; threading is controlled by the underlying Barretenberg/WASM setup. Keeping this can be ignored or cause confusion at best.Apply:
- const backend = new UltraHonkBackend(circuit.bytecode, { threads: 4 }); + const backend = new UltraHonkBackend(circuit.bytecode);examples/CRISP/apps/server/src/server/routes/voting.rs (1)
71-77: Don’tunwrap()on hex decode; return an HTTP error instead.
unwrap()will panic on invalid input. Handle the error and return a response.Apply:
- // Decode hex string to Vec<u8>, then convert to Bytes - let input_bytes = hex::decode(vote.crisp_inputs.strip_prefix("0x").unwrap_or(&vote.crisp_inputs)) - .map_err(|e| { - log::error!("Failed to decode hex string: {:?}", e); - HttpResponse::InternalServerError().json("Invalid hex string") - }).unwrap(); + // Decode hex string to Vec<u8>, then convert to Bytes + let input_bytes = match hex::decode(vote.crisp_inputs.strip_prefix("0x").unwrap_or(&vote.crisp_inputs)) { + Ok(bytes) => bytes, + Err(e) => { + log::error!("Failed to decode hex string: {:?}", e); + return HttpResponse::InternalServerError().json("Invalid hex string"); + } + };crates/wasm/src/lib.rs (2)
7-9: Import Uint8Array for proper Vec → JS conversion.
Vec<u8>cannot be directly converted toJsValue. Usejs_sys::Uint8Array.-use e3_bfv_helpers::client::{bfv_encrypt_u64, bfv_verifiable_encrypt_u64}; -use wasm_bindgen::prelude::*; +use e3_bfv_helpers::client::{bfv_encrypt_u64, bfv_verifiable_encrypt_u64}; +use wasm_bindgen::prelude::*; +use js_sys::Uint8Array;
69-74: Fix JsValue conversions: use Uint8Array for bytes and from_str for strings.Current conversions won’t compile for
Vec<u8>and are lossy/incorrect forString.- // Return as a vector of JsValues - Ok(vec![ - JsValue::from(result.encrypted_vote), - JsValue::from(result.circuit_inputs), - ]) + // Return as a vector of JsValues [Uint8Array, string] + Ok(vec![ + Uint8Array::from(result.encrypted_vote.as_slice()).into(), + JsValue::from_str(&result.circuit_inputs), + ])examples/CRISP/test/crisp.contracts.test.ts (3)
37-38: Fix invalid TypeScript tuple syntax for emojis.
string[2]is invalid. Use a tuple type.- emojis: string[2] + emojis: [string, string]
92-101: Remove hardcoded private key; read from env and default rpcUrl.Avoid committing secrets and keep network settings consistent.
- const sdk = EnclaveSDK.create({ - rpcUrl: "http://localhost:8545", + const sdk = EnclaveSDK.create({ + rpcUrl: process.env.RPC_URL ?? "http://localhost:8545", contracts: { enclave: enclaveAddress, ciphernodeRegistry: "0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0" }, chainId: 31337, protocol: FheProtocol.BFV, - privateKey: "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" + // Prefer not committing keys; rely on env or omit if not required. + privateKey: process.env.TEST_PRIVATE_KEY as `0x${string}` | undefined })I can add .env.example entries and docs if you want.
169-171: Wait for registration tx to mine to avoid flakiness.
getGroupMembersor subsequent proof gen may race without confirmation.- // register the member - await crispProgramContract.registerMember(roundId, semaphoreIdentity.commitment) + // register the member and wait for mining + await (await crispProgramContract.registerMember(roundId, semaphoreIdentity.commitment)).wait()crates/bfv-helpers/src/client.rs (1)
25-27: Docstring return type is incorrect (should be Result<Vec>).This function returns
anyhow::Result<Vec<u8>>, notResult<VerifiableEncryptionResult, String>. Also, we don't return aStringerror type anywhere in this module.Apply this diff to fix the docs:
/// # Returns -/// * `Result<VerifiableEncryptionResult, String>` - Contains encrypted vote and circuit inputs for ZKP +/// * `Result<Vec<u8>>` - Serialized BFV ciphertext bytespackages/enclave-sdk/src/enclave-sdk.ts (1)
148-168: Wrap BFV case body in braces to satisfy noSwitchDeclarations and prevent scope leakage.Local declarations (
encryptedVote,circuitInputs,inputs,proof) should be block-scoped.- switch (this.protocol) { - case FheProtocol.BFV: - const [encryptedVote, circuitInputs] = bfv_verifiable_encrypt_number( - data, - publicKey, - this.protocolParams.degree, - this.protocolParams.plaintextModulus, - this.protocolParams.moduli, - ); - - const inputs = JSON.parse(circuitInputs) as CircuitInputs; - inputs.params = defaultParams; - const proof = await generateProof(inputs, circuit); - - return { - encryptedVote, - proof, - }; + switch (this.protocol) { + case FheProtocol.BFV: { + const [encryptedVote, circuitInputs] = bfv_verifiable_encrypt_number( + data, + publicKey, + this.protocolParams.degree, + this.protocolParams.plaintextModulus, + this.protocolParams.moduli, + ); + const inputs = JSON.parse(circuitInputs) as CircuitInputs; + inputs.params = defaultParams; + const proof = await generateProof(inputs, circuit); + return { encryptedVote, proof }; + } default: throw new Error("Protocol not supported") }
🧹 Nitpick comments (20)
crates/wasm/Cargo.toml (2)
12-16: Consider centralizing wasm-bindgen under workspace.dependencies for consistency.To avoid version skew across crates and simplify upgrades, pin
wasm-bindgenonce at the workspace level and reference it here viaworkspace = true.Apply this change in this file:
[dependencies] e3-bfv-helpers.workspace = true getrandom = { version = "0.2", features = ["js"] } -wasm-bindgen = "0.2.99" +wasm-bindgen.workspace = trueThen, in your root Cargo.toml (workspace), add or update:
[workspace.dependencies] wasm-bindgen = "0.2.99"
15-15: Synchronize wasm-bindgen patch version with your CLI/tooling
crates/wasm/Cargo.toml:15 currently declares
wasm-bindgen = "0.2.99"but Cargo.lock has resolved it to 0.2.100 (allowed by the default caret (
^0.2.99) requirement). If your installedwasm-bindgen-cliis still at 0.2.99, you may see binding-generation errors.Verify your CLI/tooling versions
- Run
wasm-bindgen --version(should report 0.2.100)- If you’re using wasm-pack (see
templates/default/client/package.jsonwhich pins"wasm-pack": "^0.13.1"), runwasm-pack --versionto ensure it bundles the same wasm-bindgen patch.Optional refactor: to lock in the exact patch, update the dependency to:
--- a/crates/wasm/Cargo.toml +++ b/crates/wasm/Cargo.toml @@ -13,3 +13,3 @@ getrandom = { version = "0.2", features = ["js"] } -wasm-bindgen = "0.2.99" +wasm-bindgen = "0.2.100"This makes your Cargo.toml and Cargo.lock consistent and avoids unexpected patch bumps.
examples/CRISP/.env.example (1)
1-3: Align .env template with dotenv conventions; add missing placeholders used elsewhere
- Remove quotes (dotenv-linter warning), add trailing newline, and alphabetize keys.
- If INFURA_API_KEY is referenced in the config, include it here to reduce setup friction.
Proposed patch:
-PRIVATE_KEY="" -MNEMONIC="" -RPC_URL="http://localhost:8545" +MNEMONIC= +PRIVATE_KEY= +RPC_URL=http://localhost:8545 +# INFURA_API_KEY= +If INFURA_API_KEY is indeed required by examples/CRISP/hardhat.config.ts, I recommend adding it now. Otherwise, skip that line.
crates/wasm/init_node.js (1)
7-9: Keep initializer signature consistent with web entry pointThe web initializer accepts an initParams argument and returns a Promise. Mirroring that here avoids TS/IDE signature drift and simplifies cross-env usage.
Apply:
-export default async function initializeWasm() { - // Node does not need to be loaded async -} +export default async function initializeWasm(_initParams?: unknown) { + // Node does not need to be loaded asynchronously; no-op to keep API parity. + return; +}crates/wasm/init_web.js (1)
8-10: Drop redundant awaitReturning the Promise directly is equivalent and avoids an extra microtask.
-export default async function initializeWasm(initParams) { - return await init(initParams); -} +export default async function initializeWasm(initParams) { + return init(initParams); +}packages/enclave-sdk/package.json (1)
9-15: Exports + main/module look good; consider adding sideEffects flagThe export map and dual CJS/ESM entry points are coherent. If the package has no global side effects, adding sideEffects: false enables better tree-shaking.
"types": "./dist/index.d.ts", + "sideEffects": false,packages/enclave-sdk/src/types.ts (1)
49-57: SDKConfig: new required protocol field may be a breaking changeMaking protocol mandatory is likely correct for the new design, but it will break existing call sites. If you intend BFV as the default, consider making protocol optional and defaulting to FheProtocol.BFV internally.
Option:
- protocol: FheProtocol; + protocol?: FheProtocol; // defaults to FheProtocol.BFV if omittedEnsure all EnclaveSDK.create or constructor call sites are updated accordingly or that a default is applied downstream.
packages/enclave-sdk/src/greco.ts (1)
146-176: Consider passingparamsthrough directly to reduce boilerplate.You can forward
circuitInputs.paramsdirectly if the ACIR expects the same shape. This reduces maintenance risk when params evolve.Example:
- const { witness } = await noir.execute({ - params: { - q_mod_t: circuitInputs.params.q_mod_t, - pk_bounds: circuitInputs.params.pk_bounds, - e_bound: circuitInputs.params.e_bound, - u_bound: circuitInputs.params.u_bound, - r1_low_bounds: circuitInputs.params.r1_low_bounds, - r1_up_bounds: circuitInputs.params.r1_up_bounds, - r2_bounds: circuitInputs.params.r2_bounds, - p1_bounds: circuitInputs.params.p1_bounds, - p2_bounds: circuitInputs.params.p2_bounds, - k1_low_bound: circuitInputs.params.k1_low_bound, - k1_up_bound: circuitInputs.params.k1_up_bound, - qis: circuitInputs.params.qis, - k0is: circuitInputs.params.k0is, - size: circuitInputs.params.size, - tag: circuitInputs.params.tag, - }, + const { witness } = await noir.execute({ + params: circuitInputs.params, pk0is: pk0is_poly, pk1is: pk1is_poly, ct0is: ct0is_poly, ct1is: ct1is_poly, u: u_poly, e0: e0_poly, e1: e1_poly, k1: k1_poly, r1is: r1is_poly, r2is: r2is_poly, p1is: p1is_poly, p2is: p2is_poly, });examples/CRISP/apps/server/src/server/routes/voting.rs (1)
88-92: Fix misleading log message on contract initialization failure.This branch isn’t a “Database error”; it’s a contract client initialization error. Adjusting improves observability.
- Err(e) => { - log::error!("Database error checking vote status: {:?}", e); - return HttpResponse::InternalServerError().json("Internal server error"); - } + Err(e) => { + log::error!("Failed to initialize Enclave contract client: {:?}", e); + return HttpResponse::InternalServerError().json("Internal server error"); + }crates/wasm/src/lib.rs (2)
11-21: Docstring mismatch: not using “default params”.These functions accept explicit BFV parameters; update the docs to avoid confusion.
-/// A function to encrypt a u64 value using BFV and default params. +/// Encrypt a u64 value using BFV with explicit parameters. @@ -/// * `moduli` - Modulus for BFV parameters +/// * `moduli` - Modulus for BFV parameters (single prime for this wrapper)
40-55: Optional: return a single JsValue (Array) to match “single function returning JsValue”.If you want to strictly align with the PR objective, return a single
JsValuethat is a JS Array. This is a signature change, so only do it if TS callers are updated.Diff (requires updating return type):
-/// Returns a `Result<Vec<JsValue>, JsValue>` containing the encrypted data, circuit inputs and any errors. +/// Returns a `Result<JsValue, JsValue>` containing a JS Array [encrypted data, circuit inputs] or an error. @@ -pub fn bfv_verifiable_encrypt_number( +pub fn bfv_verifiable_encrypt_number( @@ -) -> Result<Vec<JsValue>, JsValue> { +) -> Result<JsValue, JsValue> { @@ - // Return as a vector of JsValues [Uint8Array, string] - Ok(vec![ - Uint8Array::from(result.encrypted_vote.as_slice()).into(), - JsValue::from_str(&result.circuit_inputs), - ]) + let arr = js_sys::Array::new(); + arr.push(&Uint8Array::from(result.encrypted_vote.as_slice()).into()); + arr.push(&JsValue::from_str(&result.circuit_inputs)); + Ok(arr.into())packages/enclave-sdk/tests/sdk.test.ts (2)
32-40: Typo: “environent” → “environment”.Minor nit in test description.
- it("should encrypt a number without crashing in a node environent", async () => { + it("should encrypt a number without crashing in a node environment", async () => { @@ - // TODO: test the encryption is correct + // TODO: test the encryption determinism/correctness if appropriate
38-39: Brittle assertion on ciphertext length.Hard-coding
27_674ties the test to a specific BFV parameterization/version. If params change, this will fail even though the function is correct.Suggested alternative:
- expect(value.length).to.equal(27_674); + expect(value.length).to.be.greaterThan(0);crates/bfv-helpers/src/client.rs (4)
21-24: Parameter docs are inconsistent with the actual types.
- Use the same bullet style for all parameters.
moduliis[u64; 1]here (single modulus), not a “Vector of moduli”.-/// # `degree` - Polynomial degree for BFV parameters -/// # `plaintext_modulus` - Plaintext modulus for BFV parameters -/// * `moduli` - Vector of moduli for BFV parameters +/// * `degree` - Polynomial degree for BFV parameters +/// * `plaintext_modulus` - Plaintext modulus for BFV parameters +/// * `moduli` - RNS moduli array for BFV parameters (fixed-size: 1)
64-76: Docstring return type should reflect anyhow::Result and parameter bullets.You return
anyhow::Result<VerifiableEncryptionResult>; the doc currently mentionsResult<VerifiableEncryptionResult, String>. Also align parameter bullet styling and clarifymoduli./// # Returns -/// * `Result<VerifiableEncryptionResult, String>` - Contains encrypted vote and circuit inputs for ZKP +/// * `Result<VerifiableEncryptionResult>` - Contains encrypted vote and circuit inputs for ZKPAnd consider mirroring the earlier param bullet fix here as well.
116-123: Prefer a module-level constant for the ZKP modulus; avoidunwrap().The modulus is a well-known constant. Consider hoisting it to a
const &strand parsing once (e.g., lazy_static) or documenting whyunwrap()is safe here.- let zkp_modulus = BigInt::from_str_radix( - "21888242871839275222246405745257275088548364400416034343698204186575808495617", - 10, - ) - .unwrap(); + // Safe unwrap: compile-time constant string for BN254 field modulus + let zkp_modulus = BigInt::from_str_radix( + "21888242871839275222246405745257275088548364400416034343698204186575808495617", + 10, + ).expect("invalid BN254 modulus literal");
130-177: Tests cover happy paths; consider adding failure-path tests.Current tests validate round-trip decrypt for both simple and verifiable encryption. Consider adding tests for:
- Bad public key deserialization
- Encoding failure due to params mismatch
I can add negative tests against malformed inputs if you want a follow-up patch.
examples/CRISP/contracts/CRISPVerifier.sol (3)
480-517: Transcript round-0: split public inputs vs pairing points is correct, but keep TODO tracked.You subtract
PAIRING_POINTS_SIZEwhen iterating public inputs and then append the pairing points. This matches the length check inverify().Consider referencing the TODO with an issue link or clarifying comment on why
publicInputsSizeincludes pairing points, to avoid future regressions.
679-739: loadProof layout matches the extended proof; consider a compile-time length assertion.The deserialization order reflects the new pairing point object and commitment ordering. To harden against drift, consider adding an internal check that
boundary == PROOF_SIZE * 32at the end.function loadProof(bytes calldata proof) internal pure returns (Honk.Proof memory p) { @@ - p.kzgQuotient = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + p.kzgQuotient = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + // Optional: assert we've consumed exactly PROOF_SIZE * 32 bytes + require(boundary + 0x80 == PROOF_SIZE * 32, "proof length mismatch"); }
770-792: Pairing precompile usage is unchanged; consider reverting to revert-on-failure for clarity.You return
success && decodedResult. It’s fine, but reverting on precompile failure can be more explicit if you don’t intend to handlefalse.- (bool success, bytes memory result) = address(0x08).staticcall(input); - bool decodedResult = abi.decode(result, (bool)); - return success && decodedResult; + (bool success, bytes memory result) = address(0x08).staticcall(input); + require(success, "pairing precompile failed"); + return abi.decode(result, (bool));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (5)
Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasmis excluded by!**/*.wasmexamples/CRISP/apps/server/Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/wasm-crypto/Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (46)
crates/bfv-helpers/Cargo.toml(1 hunks)crates/bfv-helpers/src/client.rs(2 hunks)crates/bfv-helpers/src/lib.rs(1 hunks)crates/bfv-helpers/src/util.rs(0 hunks)crates/evm-helpers/src/contracts.rs(1 hunks)crates/wasm/Cargo.toml(1 hunks)crates/wasm/README.md(1 hunks)crates/wasm/init_node.cjs(1 hunks)crates/wasm/init_node.js(1 hunks)crates/wasm/init_web.js(1 hunks)crates/wasm/package.json(2 hunks)crates/wasm/src/lib.rs(1 hunks)examples/CRISP/.env.example(1 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.js(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js(1 hunks)examples/CRISP/apps/client/package.json(1 hunks)examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts(3 hunks)examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx(1 hunks)examples/CRISP/apps/client/src/model/vote.model.ts(1 hunks)examples/CRISP/apps/client/src/utils/proof-encoding.ts(1 hunks)examples/CRISP/apps/client/src/utils/proofUtils.ts(0 hunks)examples/CRISP/apps/server/src/server/mod.rs(1 hunks)examples/CRISP/apps/server/src/server/models.rs(1 hunks)examples/CRISP/apps/server/src/server/routes/voting.rs(3 hunks)examples/CRISP/apps/wasm-crypto/Cargo.toml(0 hunks)examples/CRISP/apps/wasm-crypto/README.md(0 hunks)examples/CRISP/apps/wasm-crypto/src/lib.rs(0 hunks)examples/CRISP/contracts/CRISPVerifier.sol(12 hunks)examples/CRISP/hardhat.config.ts(2 hunks)examples/CRISP/package.json(3 hunks)examples/CRISP/test/crisp.contracts.test.ts(1 hunks)examples/CRISP/tsconfig.json(1 hunks)packages/enclave-react/src/useEnclaveSDK.ts(4 hunks)packages/enclave-sdk/package.json(2 hunks)packages/enclave-sdk/src/enclave-sdk.ts(6 hunks)packages/enclave-sdk/src/greco.ts(1 hunks)packages/enclave-sdk/src/index.ts(2 hunks)packages/enclave-sdk/src/types.ts(2 hunks)packages/enclave-sdk/src/wasm.ts(0 hunks)packages/enclave-sdk/tests/sdk.test.ts(1 hunks)packages/enclave-sdk/tsup.config.js(1 hunks)packages/evm/package.json(1 hunks)templates/default/client/src/pages/WizardSDK.tsx(2 hunks)templates/default/tests/integration.spec.ts(3 hunks)
💤 Files with no reviewable changes (9)
- crates/bfv-helpers/src/util.rs
- packages/enclave-sdk/src/wasm.ts
- examples/CRISP/apps/client/src/utils/proofUtils.ts
- examples/CRISP/apps/wasm-crypto/Cargo.toml
- examples/CRISP/apps/wasm-crypto/README.md
- examples/CRISP/apps/wasm-crypto/src/lib.rs
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.js
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts
🚧 Files skipped from review as they are similar to previous changes (23)
- examples/CRISP/apps/client/package.json
- crates/bfv-helpers/src/lib.rs
- examples/CRISP/apps/server/src/server/mod.rs
- packages/enclave-sdk/src/index.ts
- crates/bfv-helpers/Cargo.toml
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js
- examples/CRISP/apps/client/src/utils/proof-encoding.ts
- crates/evm-helpers/src/contracts.rs
- examples/CRISP/apps/server/src/server/models.rs
- templates/default/client/src/pages/WizardSDK.tsx
- examples/CRISP/tsconfig.json
- examples/CRISP/apps/client/src/model/vote.model.ts
- crates/wasm/init_node.cjs
- crates/wasm/package.json
- packages/evm/package.json
- examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx
- packages/enclave-sdk/tsup.config.js
- examples/CRISP/hardhat.config.ts
- crates/wasm/README.md
- templates/default/tests/integration.spec.ts
- examples/CRISP/package.json
- examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts
- packages/enclave-react/src/useEnclaveSDK.ts
🧰 Additional context used
🧬 Code Graph Analysis (9)
crates/wasm/init_node.js (1)
crates/wasm/init_web.js (1)
initializeWasm(8-10)
crates/wasm/init_web.js (1)
crates/wasm/init_node.js (1)
initializeWasm(7-9)
crates/wasm/src/lib.rs (1)
crates/bfv-helpers/src/client.rs (2)
bfv_encrypt_u64(34-56)bfv_verifiable_encrypt_u64(83-128)
crates/bfv-helpers/src/client.rs (3)
crates/bfv-helpers/src/lib.rs (1)
build_bfv_params_arc(76-90)packages/enclave-sdk/src/types.ts (1)
VerifiableEncryptionResult(256-265)crates/fhe/src/fhe.rs (1)
from_bytes(169-171)
packages/enclave-sdk/src/greco.ts (1)
packages/enclave-sdk/src/index.ts (1)
generateProof(67-67)
packages/enclave-sdk/tests/sdk.test.ts (2)
examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (1)
sdk(18-28)packages/enclave-sdk/src/enclave-sdk.ts (1)
EnclaveSDK(42-474)
examples/CRISP/apps/server/src/server/routes/voting.rs (1)
examples/CRISP/apps/server/src/server/models.rs (2)
from(149-159)from(163-177)
examples/CRISP/test/crisp.contracts.test.ts (2)
examples/CRISP/apps/client/src/utils/proof-encoding.ts (1)
encodeSemaphoreProof(14-29)packages/enclave-sdk/src/enclave-sdk.ts (1)
EnclaveSDK(42-474)
packages/enclave-sdk/src/enclave-sdk.ts (5)
packages/enclave-sdk/src/types.ts (3)
ProtocolParams(284-297)BfvProtocolParams(302-314)VerifiableEncryptionResult(256-265)crates/wasm/init_node.js (1)
initializeWasm(7-9)crates/wasm/init_web.js (1)
initializeWasm(8-10)crates/wasm/src/lib.rs (2)
bfv_encrypt_number(28-38)bfv_verifiable_encrypt_number(59-74)packages/enclave-sdk/src/greco.ts (3)
CircuitInputs(23-37)defaultParams(81-97)generateProof(128-179)
🪛 dotenv-linter (3.3.0)
examples/CRISP/.env.example
[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [UnorderedKey] The MNEMONIC key should go before the PRIVATE_KEY key
(UnorderedKey)
[warning] 3-3: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 Gitleaks (8.27.2)
examples/CRISP/test/crisp.contracts.test.ts
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Biome (2.1.2)
packages/enclave-sdk/src/enclave-sdk.ts
[error] 149-155: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 157-157: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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). (10)
- GitHub Check: build_enclave_cli
- GitHub Check: build_sdk
- GitHub Check: test_net
- GitHub Check: integration_prebuild
- GitHub Check: rust_unit
- GitHub Check: test_contracts
- GitHub Check: build_e3_support_risc0
- GitHub Check: e2e
- GitHub Check: Build & Push Image
- GitHub Check: Build & Push Image
🔇 Additional comments (14)
packages/enclave-sdk/src/types.ts (2)
256-265: VerifiableEncryptionResult shape looks goodUint8Array for ciphertext and ProofData for proof match the intended usage.
8-8: ProofData export confirmed in @aztec/bb.js v0.82.2I packed and inspected the published v0.82.2 types and found:
proof/index.d.tsdeclaresexport type ProofData = { … }- Both browser and Node entrypoints re-export
ProofDatafrom./proof/index.jsYour import
import type { ProofData } from "@aztec/bb.js";is valid—no upstream name change or adjustment is required.
packages/enclave-sdk/src/greco.ts (1)
178-179: Use Keccak transcript for EVM proofs — LGTM.The
{ keccak: true }flag is correct for producing EVM‑verifiable proofs.examples/CRISP/test/crisp.contracts.test.ts (1)
175-186: ABI encoding types look correct — LGTM.Inputs layout matches the expected CRISP program signature.
crates/bfv-helpers/src/client.rs (3)
34-56: LGTM: BFV u64 encryption path is correct and idiomatic.
- Parameters built via
build_bfv_params_arclook correct.- Public key deserialization, encoding, and encryption error mapping is clean with
anyhow.- Using
thread_rng()is fine for this context.
58-63: Naming aligns with SDK API; visibility and fields are appropriate.The
VerifiableEncryptionResultmirrors the intended SDK payload shape (ciphertext bytes and circuit inputs string). Looks good.
100-115: Good: Extended encryption + Greco vector computation sequenced correctly.
try_encrypt_extendedand subsequentInputValidationVectors::computeinputs are in the expected order.- Error contexts are helpful.
packages/enclave-sdk/src/enclave-sdk.ts (2)
118-132: LGTM: BFV encryption wiring is straightforward and matches protocol params.
initializeWasm()then direct call intobfv_encrypt_numberis OK; returning the value from an async method is fine.- Param order matches the wasm binding.
79-91: Reasonable defaults: auto-select BFV params when omitted.Defaulting to
BfvProtocolParams.BFV_NORMALis a good DX improvement. Throwing for unsupported protocols is clear.examples/CRISP/contracts/CRISPVerifier.sol (5)
7-14: Public inputs size increased; ensure all dependent constants are regenerated.
NUMBER_OF_PUBLIC_INPUTSandvk.publicInputsSizeboth set to 8223. Confirm tooling regenerated VK constants consistently with this size.You can sanity-check the regenerated VK consistency by ensuring:
- All G1 points remain on-curve (if you have a curve check script).
- The proof length constant matches the serialized layout below.
If helpful, I can provide a script to cross-validate expected proof bytes length against
loadProoflayout counts.
270-273: Pairing points integration: constant and array sizing added. Looks coherent.Introducing
PAIRING_POINTS_SIZE = 16aligns with its usage across transcript and delta computations.
389-411: Proof structure extended withpairingPointObject; ordering aligns with transcript usage.
- Added field precedes wire commitments, matching the updated loader and transcript round-0 assembly.
1562-1583: Length and public inputs checks updated correctly.
PROOF_SIZE = 456is enforced.- External
publicInputslength is checked againstvk.publicInputsSize - PAIRING_POINTS_SIZE, which aligns with how you build the transcript including pairing points from the proof instead.
1600-1635: Public input delta now includes pairing points; math order looks consistent.You first fold “main” public inputs, then fold the 16 pairing points from the proof. Accumulator updates advance
numeratorAcc/denominatorAccuniformly across both loops, which is consistent.
ctrlc03
force-pushed
the
feat/wasm-crypto-to-sdk
branch
from
0aec1b4 to
bb40ee8
Compare
ctrlc03
force-pushed
the
feat/wasm-crypto-to-sdk
branch
from
bb40ee8 to
f2ab209
Compare
ctrlc03
force-pushed
the
feat/wasm-crypto-to-sdk
branch
from
f2ab209 to
b545581
Compare
ctrlc03
force-pushed
the
feat/wasm-crypto-to-sdk
branch
from
b545581 to
bc56cc7
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (9)
examples/CRISP/test/crisp.contracts.test.ts (4)
36-38: Fix invalid tuple syntax and align committee_public_key type with actual payload.
string[2]is invalid TypeScript; use a tuple[string, string].- The server returns
committee_public_keyasnumber[](JSON array of bytes), notUint8Array. Type it correctly and convert at use sites.Apply this diff:
- committee_public_key: Uint8Array - emojis: string[2] + committee_public_key: number[] + emojis: [string, string]
92-101: Remove hardcoded private key and use the env-provided rpcUrl.A private key is committed in the test (flagged by gitleaks), and the SDK ignores the
rpcUrlenv you already read at Line 20. Use the env value and avoid committing secrets.- const sdk = EnclaveSDK.create({ - rpcUrl: "http://localhost:8545", + const sdk = EnclaveSDK.create({ + rpcUrl, contracts: { enclave: enclaveAddress, ciphernodeRegistry: "0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0" }, chainId: 31337, protocol: FheProtocol.BFV, - privateKey: "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" + // Omit private keys from source. If required for this test, read from env: + // privateKey: process.env.TEST_PRIVATE_KEY as `0x${string}`, })
169-174: Wait for the registration transaction to mine before generating the proof.Without waiting,
getGroupMembersmay not see the new member, causing flaky proofs.// register the member - await crispProgramContract.registerMember(roundId, semaphoreIdentity.commitment) + await (await crispProgramContract.registerMember(roundId, semaphoreIdentity.commitment)).wait()
164-166: Convert committee_public_key to Uint8Array at use site.After changing the type to
number[], ensure you pass aUint8Arrayinto the SDK.- const publicKey = stateJson.committee_public_key + const publicKey = new Uint8Array(stateJson.committee_public_key)packages/enclave-sdk/src/types.ts (2)
284-297: Retype ProtocolParams.moduli to an array to match BFV RNS and the exampleThe JSDoc example shows an array [0x3FFFFFFF000001], and BFV typically uses multiple RNS primes. Keep the API future-proof by using a readonly bigint[].
Apply:
export interface ProtocolParams { /** * The degree of the polynomial */ degree: number; /** * The plaintext modulus */ plaintextModulus: bigint; /** - * The moduli + * The moduli (RNS primes) */ - moduli: bigint; + moduli: readonly bigint[]; }Follow-up: You will need to adapt call sites (e.g., when passing a single prime into wasm, use moduli[0]). I can help generate a patch across the repo.
#!/bin/bash # Find usages that will need adapting after retyping `ProtocolParams.moduli` to an array rg -n -C2 --type=ts '\bprotocolParams\b|\.moduli\b'
302-314: Align BFV preset with array-based moduliUpdate the preset to provide an array value so it satisfies the revised type and the doc example.
export const BfvProtocolParams = { @@ BFV_NORMAL: { degree: 2048, plaintextModulus: 1032193n, - moduli: 0x3FFFFFFF000001n, + moduli: [0x3FFFFFFF000001n], } as const satisfies ProtocolParams, }crates/wasm/src/lib.rs (1)
69-74: Fix JsValue conversion for Vec and String (current code won’t compile)Vec doesn’t implement Into. Convert to js_sys::Uint8Array. Also use JsValue::from_str for the String.
-use e3_bfv_helpers::client::{bfv_encrypt_u64, bfv_verifiable_encrypt_u64}; +use e3_bfv_helpers::client::{bfv_encrypt_u64, bfv_verifiable_encrypt_u64}; +use js_sys::Uint8Array; @@ - // Return as a vector of JsValues - Ok(vec![ - JsValue::from(result.encrypted_vote), - JsValue::from(result.circuit_inputs), - ]) + // Return as a vector of JsValues + Ok(vec![ + Uint8Array::from(result.encrypted_vote.as_slice()).into(), + JsValue::from_str(&result.circuit_inputs), + ])Optional: To align with “single function returning JsValue”, return a JsValue Array instead of Vec. I can provide a follow-up patch if you want to make that change.
crates/bfv-helpers/src/client.rs (1)
16-27: Fix doc: wrong return type and inconsistent param names (“vote” vs “data”)The doc says Result<VerifiableEncryptionResult, String>, but the function returns Result<Vec>. Also, the summary says “vote” while the parameter is named data.
-/// Encrypt a u64 vote using BFV homomorphic encryption +/// Encrypt a u64 value using BFV homomorphic encryption @@ -/// * `vote` - The vote value to encrypt (u64) +/// * `data` - The value to encrypt (u64) @@ -/// * `Result<VerifiableEncryptionResult, String>` - Contains encrypted vote and circuit inputs for ZKP +/// * `Result<Vec<u8>>` - Serialized BFV ciphertext bytes on success @@ -/// Returns error string if: +/// Returns an error if:packages/enclave-sdk/src/enclave-sdk.ts (1)
148-167: Wrap switch case body to satisfy noSwitchDeclarations and avoid scope leaksBiome flags declarations inside switch clauses; wrap the BFV case in braces.
- case FheProtocol.BFV: - const [encryptedVote, circuitInputs] = bfv_verifiable_encrypt_number( + case FheProtocol.BFV: { + const [encryptedVote, circuitInputs] = bfv_verifiable_encrypt_number( data, publicKey, this.protocolParams.degree, this.protocolParams.plaintextModulus, this.protocolParams.moduli, ); const inputs = JSON.parse(circuitInputs) as CircuitInputs; inputs.params = defaultParams; const proof = await generateProof(inputs, circuit); return { encryptedVote, proof, }; + }If you also retype ProtocolParams.moduli to an array, remember to pass this.protocolParams.moduli[0] here.
🧹 Nitpick comments (17)
examples/CRISP/.env.example (1)
1-3: Align .env.example with dotenv-linter: remove quotes, reorder keys, add trailing newlineRemoves noisy quotes, puts MNEMONIC before PRIVATE_KEY, and ensures a newline at EOF as per the linter hints.
Apply this diff:
-PRIVATE_KEY="" -MNEMONIC="" -RPC_URL="http://localhost:8545" +MNEMONIC= +PRIVATE_KEY= +RPC_URL=http://localhost:8545 +examples/CRISP/apps/server/src/server/mod.rs (4)
30-30: Initialize logger early — consider making it idempotent to avoid double-init panicsIf
start()is invoked multiple times in-process (e.g., integration tests),env_logger’sinit()can error on re-init. Makeinit_logger()no-op when already initialized (usetry_initor a Once guard).Suggested change in
examples/CRISP/apps/server/src/logger.rs:pub fn init_logger() { let mut builder = Builder::new(); builder .target(Target::Stdout) .filter(None, LevelFilter::Info) .format(|buf, record: &Record| { let file = record.file().unwrap_or("unknown"); let filename = Path::new(file).file_name().unwrap_or_else(|| file.as_ref()); let timestamp = Utc::now().format("%Y-%m-%d %H:%M:%S"); writeln!( buf, "[{}] [{}:{}] - {}", timestamp, filename.to_string_lossy(), record.line().unwrap_or(0), record.args() ) }); // Avoid panicking if already initialized (e.g., in tests) let _ = builder.try_init(); }I can push this as a follow-up change if you want.
49-49: Route errors through the logger for consistencyUse
log::error!instead ofeprintln!so errors respect your log formatting and sinks.Apply this diff:
- eprintln!("Listener failed: {:?}", e); + log::error!("Listener failed: {:?}", e);
72-72: Prefer structured logs overprintln!Emit startup message via the logger to keep formatting and log levels consistent.
Apply this diff:
- println!("'crisp-server' listening on http://{}", bind_addr); + log::info!("'crisp-server' listening on http://{}", bind_addr);
54-55: Optional: makebind_addrconfigurableHard-coding
0.0.0.0:4000can be limiting in containerized or dev setups. Consider reading from config or env (e.g.,CONFIG.server_bind_addrorSERVER_ADDR).examples/CRISP/test/crisp.contracts.test.ts (3)
121-121: Avoid async describe callbacks in Mocha.Mocha does not await
describecallbacks. Make only the hooks/tests async.-describe("CRISP contracts", () => { - describe("voting", async () => { +describe("CRISP contracts", () => { + describe("voting", () => {
102-104: Make the server URL configurable via env for parity across environments.- const server = "http://localhost:4000" + const server = process.env.CRISP_SERVER_URL ?? "http://localhost:4000"
185-186: Optionally wait for publishInput to be mined to avoid race conditions in E2E.If later steps or external consumers depend on the published input being finalized, wait for the receipt.
- await enclaveContract.publishInput(roundId, encodedInputs) + await (await enclaveContract.publishInput(roundId, encodedInputs)).wait()templates/default/server/index.ts (1)
45-55: Consider making protocol configurable via envTo keep the template flexible, consider sourcing the protocol from configuration (e.g., env or CLI) and defaulting to BFV. This makes it easier to test future protocols without code changes.
Example approach:
- Read a
FHE_PROTOCOLenv var (e.g., "BFV") and map toFheProtocol.- Fallback to
FheProtocol.BFVwhen not provided.examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (1)
16-29: Parameterize worker config via message payloadIf this worker is used across environments (local, testnet, mainnet), accept
sdkConfigin the message and fall back to defaults only when not provided. This keeps the example portable and avoids hard-coded addresses.Example within these lines:
- const sdk = EnclaveSDK.create({ - chainId: 31337, - contracts: { - enclave: "0x...", - ciphernodeRegistry: "0x...", - }, - rpcUrl: "http://127.0.0.1:8545", - protocol: FheProtocol.BFV, - }); + const { sdkConfig } = data || {}; + const sdk = EnclaveSDK.create({ + chainId: sdkConfig?.chainId ?? 31337, + contracts: { + enclave: sdkConfig?.contracts?.enclave ?? "0x...", + ciphernodeRegistry: sdkConfig?.contracts?.ciphernodeRegistry ?? "0x...", + }, + rpcUrl: sdkConfig?.rpcUrl ?? "http://127.0.0.1:8545", + protocol: FheProtocol.BFV, + });templates/default/tests/integration.spec.ts (1)
232-234: Use viem’s bytesToHex for clarityMinor readability: replace manual hex encoding with
bytesToHexfrom viem.Apply this diff:
- const enc1 = await sdk.encryptNumber(num1, publicKeyBytes); - const enc2 = await sdk.encryptNumber(num2, publicKeyBytes); + const enc1 = await sdk.encryptNumber(num1, publicKeyBytes); + const enc2 = await sdk.encryptNumber(num2, publicKeyBytes); @@ - await sdk.publishInput( - e3Id, - `0x${Array.from(enc1, (b) => b.toString(16).padStart(2, "0")).join("")}` as `0x${string}`, - ); + await sdk.publishInput(e3Id, bytesToHex(enc1) as `0x${string}`); @@ - const hash2 = await sdk.publishInput( - e3Id, - `0x${Array.from(enc2, (b) => b.toString(16).padStart(2, "0")).join("")}` as `0x${string}`, - ); + const hash2 = await sdk.publishInput(e3Id, bytesToHex(enc2) as `0x${string}`);Add at top with existing imports:
import { hexToBytes, bytesToHex } from "viem";Also applies to: 238-246
crates/wasm/src/lib.rs (2)
11-21: Doc nit: remove “default params” wording and clarify ‘moduli’ vs ‘modulus’The function requires explicit params; also the argument name is singular (moduli: u64) but the doc says “Modulus” vs “Moduli” inconsistently.
-/// A function to encrypt a u64 value using BFV and default params. +/// Encrypt a u64 value using BFV with the specified parameters. @@ -/// * `moduli` - Modulus for BFV parameters +/// * `modulus` - Single RNS prime for BFV parametersIf you plan to support multiple primes soon, consider accepting an array and forward to the helper accordingly.
44-55: Doc nit: return type description can be tighter“Returns a Result<Vec, JsValue>” is already declared by the signature; consider focusing on payload semantics.
-/// Returns a `Result<Vec<u8>, JsValue>` containing the encrypted data and any errors. +/// On success, returns the serialized BFV ciphertext bytes.crates/bfv-helpers/src/client.rs (1)
70-76: Doc nit: “Vector of moduli” is misleading (function takes [u64; 1])The function accepts a single-prime array [u64; 1]. Clarify to avoid confusion for callers.
-/// * `moduli` - Vector of moduli for BFV parameters +/// * `moduli` - Single-prime array for BFV parameters (length = 1)examples/CRISP/contracts/CRISPVerifier.sol (3)
7-14: Avoid duplicated sources of truth for public inputs countYou set NUMBER_OF_PUBLIC_INPUTS to 8223 (Line 7) and also hardcode vk.publicInputsSize to 8223 (Line 13). Keeping these in sync is error-prone.
Prefer deriving loop bounds and checks from vk.publicInputsSize or from publicInputs.length to remove duplication. Minimal change suggestion for the delta computation is below.
Apply this diff to make computePublicInputDelta independent of the duplicated constant:
- { - for (uint256 i = 0; i < numPublicInputs - PAIRING_POINTS_SIZE; i++) { + { + // Use the actual array length to avoid mismatch with duplicated constants. + for (uint256 i = 0; i < publicInputs.length; i++) { Fr pubInput = FrLib.fromBytes32(publicInputs[i]); numerator = numerator * (numeratorAcc + pubInput); denominator = denominator * (denominatorAcc + pubInput); numeratorAcc = numeratorAcc + beta; denominatorAcc = denominatorAcc - beta; }
272-273: Explicitly document PAIRING_POINTS_SIZE contract with proof producersPAIRING_POINTS_SIZE = 16 is clear, but its semantics and ordering in the transcript/proof must match off-chain producers. A short comment stating what these 16 Frs represent and their position in the byte-serialized proof will help future maintainers keep encoders/decoders aligned.
1600-1635: Compute delta loops: correct sequencing; consider avoiding memory copy of publicInputsThe two loops cover exactly numPublicInputs elements by first iterating over user-provided publicInputs and then over the PAIRING_POINTS_SIZE items. The accumulator updates are continuous and correct.
Minor gas optimization: computePublicInputDelta currently takes bytes32[] memory publicInputs, causing a full copy from calldata. Since verify takes calldata, consider changing computePublicInputDelta to accept bytes32[] calldata to avoid the copy (supported in recent Solidity versions). If you prefer to keep it as-is for compatibility, the earlier suggestion to use publicInputs.length already reduces coupling.
Option A (preferred if compiler version allows calldata here):
- function computePublicInputDelta(bytes32[] memory publicInputs, Fr[PAIRING_POINTS_SIZE] memory pairingPointObject, Fr beta, Fr gamma, uint256 offset) + function computePublicInputDelta(bytes32[] calldata publicInputs, Fr[PAIRING_POINTS_SIZE] memory pairingPointObject, Fr beta, Fr gamma, uint256 offset) internal view returns (Fr publicInputDelta)Option B (already proposed): use publicInputs.length for the first loop to avoid relying on numPublicInputs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (5)
Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasmis excluded by!**/*.wasmexamples/CRISP/apps/server/Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/wasm-crypto/Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (47)
crates/bfv-helpers/Cargo.toml(1 hunks)crates/bfv-helpers/src/client.rs(2 hunks)crates/bfv-helpers/src/lib.rs(1 hunks)crates/bfv-helpers/src/util.rs(0 hunks)crates/evm-helpers/src/contracts.rs(1 hunks)crates/wasm/Cargo.toml(1 hunks)crates/wasm/README.md(1 hunks)crates/wasm/init_node.cjs(1 hunks)crates/wasm/init_node.js(1 hunks)crates/wasm/init_web.js(1 hunks)crates/wasm/package.json(2 hunks)crates/wasm/src/lib.rs(1 hunks)examples/CRISP/.env.example(1 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.js(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js(1 hunks)examples/CRISP/apps/client/package.json(1 hunks)examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts(3 hunks)examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx(1 hunks)examples/CRISP/apps/client/src/model/vote.model.ts(1 hunks)examples/CRISP/apps/client/src/utils/proof-encoding.ts(1 hunks)examples/CRISP/apps/client/src/utils/proofUtils.ts(0 hunks)examples/CRISP/apps/server/src/server/mod.rs(1 hunks)examples/CRISP/apps/server/src/server/models.rs(1 hunks)examples/CRISP/apps/server/src/server/routes/voting.rs(3 hunks)examples/CRISP/apps/wasm-crypto/Cargo.toml(0 hunks)examples/CRISP/apps/wasm-crypto/README.md(0 hunks)examples/CRISP/apps/wasm-crypto/src/lib.rs(0 hunks)examples/CRISP/contracts/CRISPVerifier.sol(12 hunks)examples/CRISP/hardhat.config.ts(2 hunks)examples/CRISP/package.json(3 hunks)examples/CRISP/test/crisp.contracts.test.ts(1 hunks)examples/CRISP/tsconfig.json(1 hunks)packages/enclave-react/src/useEnclaveSDK.ts(4 hunks)packages/enclave-sdk/package.json(2 hunks)packages/enclave-sdk/src/enclave-sdk.ts(6 hunks)packages/enclave-sdk/src/greco.ts(1 hunks)packages/enclave-sdk/src/index.ts(2 hunks)packages/enclave-sdk/src/types.ts(2 hunks)packages/enclave-sdk/src/wasm.ts(0 hunks)packages/enclave-sdk/tests/sdk.test.ts(1 hunks)packages/enclave-sdk/tsup.config.js(1 hunks)packages/evm/package.json(1 hunks)templates/default/client/src/pages/WizardSDK.tsx(2 hunks)templates/default/server/index.ts(2 hunks)templates/default/tests/integration.spec.ts(3 hunks)
💤 Files with no reviewable changes (9)
- crates/bfv-helpers/src/util.rs
- packages/enclave-sdk/src/wasm.ts
- examples/CRISP/apps/wasm-crypto/README.md
- examples/CRISP/apps/client/src/utils/proofUtils.ts
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.js
- examples/CRISP/apps/wasm-crypto/src/lib.rs
- examples/CRISP/apps/wasm-crypto/Cargo.toml
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts
🚧 Files skipped from review as they are similar to previous changes (25)
- crates/wasm/init_node.cjs
- crates/bfv-helpers/src/lib.rs
- packages/evm/package.json
- examples/CRISP/tsconfig.json
- crates/wasm/init_web.js
- examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx
- packages/enclave-sdk/package.json
- examples/CRISP/hardhat.config.ts
- crates/wasm/init_node.js
- examples/CRISP/apps/client/src/model/vote.model.ts
- packages/enclave-react/src/useEnclaveSDK.ts
- examples/CRISP/apps/server/src/server/routes/voting.rs
- examples/CRISP/apps/server/src/server/models.rs
- packages/enclave-sdk/tsup.config.js
- packages/enclave-sdk/tests/sdk.test.ts
- crates/evm-helpers/src/contracts.rs
- templates/default/client/src/pages/WizardSDK.tsx
- packages/enclave-sdk/src/index.ts
- examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts
- crates/bfv-helpers/Cargo.toml
- packages/enclave-sdk/src/greco.ts
- examples/CRISP/package.json
- crates/wasm/README.md
- crates/wasm/package.json
- examples/CRISP/apps/client/src/utils/proof-encoding.ts
🧰 Additional context used
🧬 Code Graph Analysis (7)
examples/CRISP/apps/server/src/server/mod.rs (1)
examples/CRISP/apps/server/src/logger.rs (1)
init_logger(13-33)
templates/default/server/index.ts (1)
packages/enclave-sdk/src/index.ts (1)
FheProtocol(42-42)
examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (1)
packages/enclave-sdk/src/enclave-sdk.ts (1)
EnclaveSDK(42-474)
packages/enclave-sdk/src/enclave-sdk.ts (6)
packages/enclave-sdk/src/index.ts (5)
FheProtocol(42-42)ProtocolParams(37-37)BfvProtocolParams(42-42)VerifiableEncryptionResult(38-38)generateProof(67-67)packages/enclave-sdk/src/types.ts (3)
ProtocolParams(284-297)BfvProtocolParams(302-314)VerifiableEncryptionResult(256-265)crates/wasm/init_node.js (1)
initializeWasm(7-9)crates/wasm/init_web.js (1)
initializeWasm(8-10)crates/wasm/src/lib.rs (2)
bfv_encrypt_number(28-38)bfv_verifiable_encrypt_number(59-74)packages/enclave-sdk/src/greco.ts (3)
CircuitInputs(23-37)defaultParams(81-97)generateProof(128-179)
crates/bfv-helpers/src/client.rs (4)
crates/bfv-helpers/src/lib.rs (1)
build_bfv_params_arc(76-90)packages/enclave-sdk/src/index.ts (1)
VerifiableEncryptionResult(38-38)packages/enclave-sdk/src/types.ts (1)
VerifiableEncryptionResult(256-265)crates/fhe/src/fhe.rs (1)
from_bytes(169-171)
templates/default/tests/integration.spec.ts (3)
packages/enclave-sdk/src/index.ts (2)
FheProtocol(42-42)EnclaveEventType(42-42)examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (1)
sdk(18-28)packages/enclave-react/src/index.ts (1)
EnclaveEventType(32-32)
crates/wasm/src/lib.rs (1)
crates/bfv-helpers/src/client.rs (2)
bfv_encrypt_u64(34-56)bfv_verifiable_encrypt_u64(83-128)
🪛 Biome (2.1.2)
packages/enclave-sdk/src/enclave-sdk.ts
[error] 149-155: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 157-157: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 dotenv-linter (3.3.0)
examples/CRISP/.env.example
[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [UnorderedKey] The MNEMONIC key should go before the PRIVATE_KEY key
(UnorderedKey)
[warning] 3-3: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 Gitleaks (8.27.2)
examples/CRISP/test/crisp.contracts.test.ts
100-100: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (18)
examples/CRISP/.env.example (1)
1-2: .env is ignored and .env.example is tracked—no changes needed
- examples/CRISP/.env is ignored by
examples/CRISP/.gitignore(*.env)- examples/CRISP/.env.example is not ignored and will be committed
examples/CRISP/apps/server/src/server/mod.rs (2)
26-26: Importing and using the centralized logger is a good moveBringing
init_loggerinto the server module aligns initialization across the app.
57-63: CORS configuration needs your confirmation
I ran a search for any session- or cookie-based authentication in the codebase and didn’t find evidence of credential-based middleware (e.g.actix_sessionor similar wrappers). Browsers will reject a wildcard origin whenAccess-Control-Allow-Credentials: true. Please confirm your client’s needs:• If you do rely on cookies or other credentials:
Replace.allow_any_origin()with an origin-echo (or allow-list) function:- let cors = Cors::default() - .allow_any_origin() + let cors = Cors::default() + // Echo incoming Origin to stay compatible with credentials. + // Consider tightening this to a config-driven allow-list. + .allowed_origin_fn(|_origin, _req_head| true) .allowed_methods(vec!["GET", "POST", "OPTIONS"]) .allow_any_header() .supports_credentials() .max_age(3600);• If you do not need credentials:
Simply drop.supports_credentials():let cors = Cors::default() .allow_any_origin() .allowed_methods(vec!["GET", "POST", "OPTIONS"]) .allow_any_header() - .supports_credentials() .max_age(3600);Please let me know which path reflects your usage so we can lock this down safely.
examples/CRISP/test/crisp.contracts.test.ts (2)
47-62: Proof encoding logic looks solid.Encoding a single tuple with viem and returning
Uint8Arrayfits the “single JsValue” binding goal and keeps boundaries clean.
74-80: Verify Group construction/import matches current semaphore libs.Double-check the
GroupAPI you’re using. Many versions expect creating an empty group with a depth (and optionally id) and then adding members; some exports come from@semaphore-protocol/grouprather than@semaphore-protocol/core.If your current API fails at runtime/compile-time, consider:
// import { Group } from '@semaphore-protocol/group' const group = new Group(merkleTreeDepth /*, groupId if required by your version */) group.addMembers(groupMembers)Or if the constructor supports members directly, confirm the signature with the installed version and keep as-is.
examples/CRISP/apps/client/package.json (1)
20-20: Enclave SDK dependency added — LGTMUsing the workspace range here is appropriate for the monorepo and will keep the client aligned with SDK changes.
templates/default/server/index.ts (1)
13-14: Protocol selection wired correctly (BFV) — LGTMImporting
FheProtocoland passingprotocol: FheProtocol.BFVtoEnclaveSDK.creatematches the new SDK API.Also applies to: 53-54
crates/wasm/Cargo.toml (1)
15-15: Pruned WASM deps — LGTMDropping unused crypto deps from the WASM layer simplifies the bundle and reduces surface area. Keeping
wasm-bindgen = "0.2.99"is fine.examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (3)
7-9: Verify JSON import in Worker contextVite typically supports JSON imports, but workers sometimes require explicit bundler configuration. Please confirm this worker entry is processed by Vite so
crisp_circuit.jsonis inlined/available at runtime.If needed, ensure the worker is created via
new URL(..., import.meta.url)and included in the Vite build graph so JSON imports are transformed.
30-35: Ensure publicKey type matches SDK expectation
encryptNumberAndGenProofexpects aUint8Arraypublic key. If callers pass a hex string, convert it before calling.Apply conversion if needed:
import { hexToBytes } from "viem"; // ... const pkBytes = typeof publicKey === "string" ? hexToBytes(publicKey) : publicKey; const result = await sdk.encryptNumberAndGenProof(voteId, pkBytes, circuit);
40-41: Confirm postMessage payload shape matches downstream consumersYou're emitting
{ encryptedVote: { vote, proofData } }. Ensure the client code is updated to expectproofDatahere (and not, e.g.,prooforcrisp_inputs). Mismatched field names will cause subtle integration failures.templates/default/tests/integration.spec.ts (2)
19-20: BFV protocol usage in tests — LGTMImporting and using
FheProtocol.BFValigns the tests with the updated SDK API.Also applies to: 170-171
160-174: Consider explicit initialization for parity with serverThe server template calls
await sdk.initialize(). If the tests rely on the same underlying setup (e.g., subscriptions), explicitly initializing here can reduce flakiness across environments.If not required, feel free to ignore; otherwise add:
await sdk.initialize();just after
EnclaveSDK.create(...).examples/CRISP/contracts/CRISPVerifier.sol (5)
390-397: Proof struct extension looks correct; verify encoder alignmentAdding pairingPointObject as Fr[16] at the top of Honk.Proof is fine, and the rest of the struct remains intact. Just ensure the off-chain encoder writes these 16 Frs first, before commitments, as loadProof now expects that ordering.
480-497: Transcript round0 extension is consistent and index-safeAppending the pairingPointObject after the first (publicInputsSize - PAIRING_POINTS_SIZE) public inputs and before w1/w2/w3 is logically sound. The array sizing and indexing are correct and avoid off-by-one errors.
1563-1571: PROOF_SIZE updated to 456: matches new layoutGiven the added 16 Fr pairing points and adjusted commitment ordering, 456 field elements (14592 bytes) is correct for the current parsing logic in loadProof. The size check will correctly reject mismatched encodings.
1576-1578: Public inputs length check accounts for pairing pointsValidating publicInputs.length == vk.publicInputsSize - PAIRING_POINTS_SIZE matches the transcript construction that treats the pairing points separately. This guards against OOB reads in subsequent loops.
1586-1588: computePublicInputDelta: callsite wiring is correctPassing both publicInputs and p.pairingPointObject into computePublicInputDelta is the right split given the new transcript layout.
ctrlc03
force-pushed
the
feat/wasm-crypto-to-sdk
branch
from
bc56cc7 to
a1cf438
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (3)
examples/CRISP/apps/server/src/server/mod.rs (1)
57-63: CORS misconfiguration: allow_any_origin + supports_credentials is invalid and insecure.Browsers reject Access-Control-Allow-Origin: * when credentials are allowed; actix-cors also disallows this combo. Either remove credentials or restrict origins.
Option A (simplest): drop credentials support.
- let cors = Cors::default() - .allow_any_origin() - .allowed_methods(vec!["GET", "POST", "OPTIONS"]) - .allow_any_header() - .supports_credentials() - .max_age(3600); + let cors = Cors::default() + .allow_any_origin() + .allowed_methods(vec!["GET", "POST", "OPTIONS"]) + .allow_any_header() + .max_age(3600);Option B (recommended if you need cookies/Authorization): restrict origins, e.g., from CONFIG.
- let cors = Cors::default() - .allow_any_origin() + let cors = Cors::default() + .allowed_origin(&CONFIG.frontend_origin) .allowed_methods(vec!["GET", "POST", "OPTIONS"]) .allow_any_header() - .supports_credentials() + .supports_credentials() .max_age(3600);crates/wasm/package.json (1)
9-12: Include root-level init files in published packageThe current
filesfield incrates/wasm/package.jsonomits the root-level init files required by the exports map, so consumers importing"@gnosis-guild/e3-wasm/init"will receive a broken package. Please update thefilesarray to explicitly include these entry points:• crates/wasm/package.json (update
filesarray)"files": [ "dist/", "dist/**/*", + "init_node.js", + "init_node.cjs", + "init_web.js", + "init.d.ts" ],All four files have been verified to exist at the package root. Ensure they’re included before publishing.
packages/enclave-sdk/package.json (1)
28-35: Move bundler/test tools to devDependencies to avoid bloating consumer installs.vite, vite plugins, and vitest should be dev-only. Currently duplicated or listed under dependencies.
Apply this diff:
"devDependencies": { "@gnosis-guild/enclave-config": "workspace:*", "concurrently": "^9.1.2", "tsup": "^8.5.0", "typescript": "5.8.3", - "vite": "^6.2.0", - "vite-plugin-dts": "^4.5.3" + "vite": "^6.2.0", + "vite-plugin-dts": "^4.5.3", + "vite-plugin-top-level-await": "^1.5.0", + "vite-plugin-wasm": "^3.4.1", + "vitest": "^1.6.1" }, "dependencies": { "@aztec/bb.js": "^0.82.2", "@noir-lang/noir_js": "1.0.0-beta.3", "@gnosis-guild/e3-wasm": "workspace:*", "@gnosis-guild/enclave": "workspace:*", "comlink": "^4.4.2", "viem": "2.30.6", - "vite-plugin-top-level-await": "^1.5.0", - "vite-plugin-wasm": "^3.4.1", - "vitest": "^1.6.1", "web-worker": "^1.5.0" }Also applies to: 37-47
♻️ Duplicate comments (13)
examples/CRISP/.env.example (1)
1-3: Add INFURA_API_KEY to align with hardhat config and testsCurrent examples refer to INFURA_API_KEY, but it’s missing from this template. Add it between MNEMONIC and RPC_URL to prevent setup failures. Consider adding a trailing newline as well.
Apply:
-PRIVATE_KEY="" -MNEMONIC="" -RPC_URL="http://localhost:8545" +PRIVATE_KEY="" +MNEMONIC="" +INFURA_API_KEY="" +RPC_URL="http://localhost:8545"packages/enclave-sdk/src/types.ts (2)
284-297: ProtocolParams.moduli should be an array of primes (RNS), not a single bigintDocs and BFV practice expect multiple moduli. Typing it as a single bigint constrains future configurations and conflicts with the docstring example.
Apply:
export interface ProtocolParams { /** * The degree of the polynomial */ degree: number; /** * The plaintext modulus */ plaintextModulus: bigint; /** - * The moduli + * The moduli (RNS primes) */ - moduli: bigint; + moduli: readonly bigint[]; }
302-314: Update default BFV params to use an array for moduliAligns with the revised ProtocolParams type and the example in the comment.
Apply:
BFV_NORMAL: { degree: 2048, plaintextModulus: 1032193n, - moduli: 0x3FFFFFFF000001n, + moduli: [0x3FFFFFFF000001n], } as const satisfies ProtocolParams,examples/CRISP/contracts/CRISPVerifier.sol (2)
519-541: Round1 challenge element order should include w4 before lookup readsThe challenge array round1 currently hashes lookupReadCounts/tags before w4. Standard layout is w1..w4, then lookup-related points, to match encoder order and maintain transcript correctness.
Apply:
- bytes32[13] memory round1; + bytes32[13] memory round1; round1[0] = FrLib.toBytes32(previousChallenge); - round1[1] = bytes32(proof.lookupReadCounts.x_0); - round1[2] = bytes32(proof.lookupReadCounts.x_1); - round1[3] = bytes32(proof.lookupReadCounts.y_0); - round1[4] = bytes32(proof.lookupReadCounts.y_1); - round1[5] = bytes32(proof.lookupReadTags.x_0); - round1[6] = bytes32(proof.lookupReadTags.x_1); - round1[7] = bytes32(proof.lookupReadTags.y_0); - round1[8] = bytes32(proof.lookupReadTags.y_1); - round1[9] = bytes32(proof.w4.x_0); - round1[10] = bytes32(proof.w4.x_1); - round1[11] = bytes32(proof.w4.y_0); - round1[12] = bytes32(proof.w4.y_1); + // Read w4 immediately after w1..w3 + round1[1] = bytes32(proof.w4.x_0); + round1[2] = bytes32(proof.w4.x_1); + round1[3] = bytes32(proof.w4.y_0); + round1[4] = bytes32(proof.w4.y_1); + // Then lookup witnesses + round1[5] = bytes32(proof.lookupReadCounts.x_0); + round1[6] = bytes32(proof.lookupReadCounts.x_1); + round1[7] = bytes32(proof.lookupReadCounts.y_0); + round1[8] = bytes32(proof.lookupReadCounts.y_1); + round1[9] = bytes32(proof.lookupReadTags.x_0); + round1[10] = bytes32(proof.lookupReadTags.x_1); + round1[11] = bytes32(proof.lookupReadTags.y_0); + round1[12] = bytes32(proof.lookupReadTags.y_1);
696-706: loadProof ordering: w4 must be read immediately after w3 (before lookup points)Current read order places lookupReadCounts/tags before w4. This likely desynchronizes with the proof encoder and the Fiat–Shamir transcript, causing verification failure.
Apply:
- p.lookupReadCounts = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); - boundary += 0x80; - p.lookupReadTags = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); - boundary += 0x80; - p.w4 = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); - boundary += 0x80; - p.lookupInverses = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); - boundary += 0x80; - p.zPerm = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); - boundary += 0x80; + // Read w4 immediately after w1..w3 + p.w4 = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + boundary += 0x80; + // Then lookup witnesses and zPerm + p.lookupReadCounts = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + boundary += 0x80; + p.lookupReadTags = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + boundary += 0x80; + p.lookupInverses = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + boundary += 0x80; + p.zPerm = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + boundary += 0x80;examples/CRISP/test/crisp.contracts.test.ts (4)
36-38: Fix State typing: committee_public_key shape and invalid tuple syntax for emojis.
- Server returns committee_public_key as JSON byte array (number[]), not Uint8Array.
- TypeScript tuple syntax
string[2]is invalid; use[string, string].Apply this diff:
interface State { id: number chain_id: number enclave_address: string status: string vote_count: number start_time: number duration: number expiration: number start_block: number - committee_public_key: Uint8Array - emojis: string[2] + committee_public_key: number[] + emojis: [string, string] }
164-165: Convert committee_public_key to Uint8Array before passing to the SDK.Runtime expects binary; convert the number[] into Uint8Array at the callsite.
Apply this diff:
- const publicKey = stateJson.committee_public_key + const publicKey = new Uint8Array(stateJson.committee_public_key)
92-101: Remove hardcoded private key and align rpcUrl with env for consistency (leaks + flakiness).Use env vars to avoid committing secrets and ensure both SDK and proof generation use the same network.
Apply this diff:
- const sdk = EnclaveSDK.create({ - rpcUrl: "http://localhost:8545", + const sdk = EnclaveSDK.create({ + rpcUrl: rpcUrl ?? "http://localhost:8545", contracts: { enclave: enclaveAddress, ciphernodeRegistry: "0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0" }, chainId: 31337, protocol: FheProtocol.BFV, - privateKey: "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" + // Optional: omit if not required for your use. Otherwise read from env. + privateKey: process.env.TEST_PRIVATE_KEY as `0x${string}`, })
169-171: Wait for transaction mining before generating membership-dependent proof.Not awaiting mining can make group membership queries stale and tests flaky.
Apply this diff:
- // register the member - await crispProgramContract.registerMember(roundId, semaphoreIdentity.commitment) + // register the member and wait for mining + await (await crispProgramContract.registerMember(roundId, semaphoreIdentity.commitment)).wait()packages/enclave-react/src/useEnclaveSDK.ts (1)
76-82: Fix doc example: wrong enum name and ProtocolParams.moduli type.
- Replace EFheProtocol.BFV with FheProtocol.BFV.
- Adjust moduli to the SDK’s type (single bigint). If the SDK truly expects multiple, switch to bigint[].
Apply this diff:
}, - protocol: EFheProtocol.BFV, + protocol: FheProtocol.BFV, protocolParams: { degree: 2048, plaintextModulus: 1032193n, - moduli: new BigUint64Array([0x3FFFFFFF000001n]), + moduli: 0x3FFFFFFF000001n, },crates/wasm/src/lib.rs (2)
71-73: Vec → JsValue conversion is invalid; convert to Uint8Array before returning
JsValue::from(result.encrypted_vote)will not compile forVec<u8>. Convert bytes tojs_sys::Uint8Arrayfirst. Also convert the string explicitly.Apply this minimal fix (keeping the current return type):
- Ok(vec![ - JsValue::from(result.encrypted_vote), - JsValue::from(result.circuit_inputs), - ]) + Ok(vec![ + js_sys::Uint8Array::from(result.encrypted_vote.as_slice()).into(), + JsValue::from_str(&result.circuit_inputs), + ])
7-9: Missing import for Uint8Array (required for byte conversion)Add the
js_sys::Uint8Array(and optionallyArray) import.-use e3_bfv_helpers::client::{bfv_encrypt_u64, bfv_verifiable_encrypt_u64}; +use e3_bfv_helpers::client::{bfv_encrypt_u64, bfv_verifiable_encrypt_u64}; +use js_sys::Uint8Array; +// If you adopt the single-JsValue Array return (see suggestion below): +// use js_sys::{Array, Uint8Array};packages/enclave-sdk/src/enclave-sdk.ts (1)
148-168: Wrap BFV case in braces to avoid leaking declarations (Biome noSwitchDeclarations)Local declarations (
encryptedVote,circuitInputs,inputs,proof) should be block-scoped to the case.- case FheProtocol.BFV: - const [encryptedVote, circuitInputs] = bfv_verifiable_encrypt_number( + case FheProtocol.BFV: { + const [encryptedVote, circuitInputs] = bfv_verifiable_encrypt_number( data, publicKey, this.protocolParams.degree, this.protocolParams.plaintextModulus, - this.protocolParams.moduli, + this.protocolParams.moduli[0], ); const inputs = JSON.parse(circuitInputs) as CircuitInputs; inputs.params = defaultParams; const proof = await generateProof(inputs, circuit); return { encryptedVote, proof, }; + }
🧹 Nitpick comments (14)
examples/CRISP/apps/server/src/server/mod.rs (3)
49-49: Use log macros instead of println!/eprintln! for consistency.Now that logging is initialized, prefer log::error!/log::info! so messages follow the configured sink/format.
- eprintln!("Listener failed: {:?}", e); + log::error!("indexer task failed: {:?}", e);- println!("'crisp-server' listening on http://{}", bind_addr); + log::info!("crisp-server listening on http://{}", bind_addr);Also applies to: 72-72
54-54: Consider making bind address configurable.Hard-coding "0.0.0.0:4000" limits deployment flexibility. Suggest sourcing from CONFIG or env with a sensible default.
29-31: Makeinit_logger()idempotent to avoid panics on double initializationWe’ve confirmed that
init_logger()is only called from two separate binaries—
examples/CRISP/apps/server/src/cli/main.rsexamples/CRISP/apps/server/src/server/mod.rsso you won’t hit a double-init at runtime today. However, because
env_logger::Builder::init()panics if invoked more than once (e.g. in tests or future extensions), it’s safer to make it idempotent. Consider one of the following optional refactors:
Switch to
try_init()and ignore the “already initialized” error:
Inexamples/CRISP/apps/server/src/logger.rspub fn init_logger() { let mut builder = Builder::new(); // … your formatting/filter setup … - builder.init(); + let _ = builder.try_init(); }Or guard the init call with a
std::sync::Once:static LOGGER_INIT: Once = Once::new(); pub fn init_logger() { LOGGER_INIT.call_once(|| { Builder::new() // … your formatting/filter setup … .init(); }); }crates/wasm/package.json (1)
18-50: Consider adding typesVersions for subpath './init' type resolutionTypeScript does not resolve "exports.types" mappings. To ensure consumers get correct d.ts for "import '@gnosis-guild/e3-wasm/init'", add a typesVersions map.
Example:
{ "name": "@gnosis-guild/e3-wasm", ... + "typesVersions": { + "*": { + "init": [ + "./init.d.ts" + ] + } + }, "exports": { ".": {packages/enclave-react/src/useEnclaveSDK.ts (1)
112-123: Include protocol fields in initializeSDK dependencies to reflect config changes.If protocol or protocolParams change, the hook won’t re-initialize.
Apply this diff:
const initializeSDK = useCallback(async () => { try { setError(null); @@ const sdkConfig: SDKConfig = { publicClient, walletClient, contracts: config.contracts || { enclave: "0x0000000000000000000000000000000000000000", ciphernodeRegistry: "0x0000000000000000000000000000000000000000", }, chainId: config.chainId, protocol: config.protocol, protocolParams: config.protocolParams, }; @@ - }, [publicClient, walletClient, config.contracts, config.chainId]); + }, [publicClient, walletClient, config.contracts, config.chainId, config.protocol, config.protocolParams]);Also applies to: 137-137
packages/enclave-sdk/package.json (1)
20-27: Monorepo prebuild path assumptions.
prebuildhops directories with relative paths. Consider using workspace scripts (pnpm -w) or referencing package scripts to reduce fragility if paths change.If desirable, I can propose a workspace-level script chain that calls
pnpm -w --filter @gnosis-guild/enclave-evm compile:ts && pnpm -w --filter @gnosis-guild/e3-wasm build.packages/enclave-sdk/tests/sdk.test.ts (2)
32-40: Typos and overly brittle assertion.
- Fix “environent” → “environment”.
- Asserting an exact ciphertext length is brittle; prefer a lower bound or structural check.
Apply this diff:
- it("should encrypt a number without crashing in a node environent", async () => { + it("should encrypt a number without crashing in a node environment", async () => { @@ - expect(value.length).to.equal(27_674); + expect(value.length).to.be.greaterThan(20_000);
41-52: Typos and timeout.
- Fix “environent” typo in the second test name too.
- Consider a more reasonable timeout (e.g., 120_000 ms) unless you know CI needs more.
Apply this diff:
- it("should encrypt a number and generate a proof without crashing in a node environent", async () => { + it("should encrypt a number and generate a proof without crashing in a node environment", async () => { @@ - }, 9999999); + }, 120_000);crates/wasm/src/lib.rs (4)
59-66: Optional: Return a single JsValue Array to align with “single function returning JsValue” goalIf you want the wasm export to be a single
JsValue, return a JS Array (js_sys::Array) instead ofVec<JsValue>and push theUint8Arrayandcircuit_inputsstring.-pub fn bfv_verifiable_encrypt_number( +pub fn bfv_verifiable_encrypt_number( @@ -) -> Result<Vec<JsValue>, JsValue> { +) -> Result<JsValue, JsValue> { @@ - // Return as a vector of JsValues - Ok(vec![ - JsValue::from(result.encrypted_vote), - JsValue::from(result.circuit_inputs), - ]) + let arr = js_sys::Array::new(); + arr.push(&Uint8Array::from(result.encrypted_vote.as_slice()).into()); + arr.push(&JsValue::from_str(&result.circuit_inputs)); + Ok(arr.into())This change matches the PR objective of simplifying to a single JsValue return while keeping the JS consumption pattern the same (array destructuring still works).
Also applies to: 69-74
11-21: Docs: “default params” wording is misleading (these functions accept explicit params)The docstring says “default params” but the function takes
degree,plaintext_modulus, andmoduli. Update wording to “explicit BFV params”.-/// A function to encrypt a u64 value using BFV and default params. +/// Encrypt a u64 value using BFV with explicit parameters. @@ -/// * `moduli` - Modulus for BFV parameters +/// * `moduli` - Modulus for BFV parameters (single RNS modulus)
25-27: Docs: “Panics” section does not reflect behaviorThe function maps errors to
JsValueand does not panic. Consider removing or rewording this section.-/// # Panics -/// -/// Panics if the data cannot be encrypted +/// # Errors +/// Returns an error as `JsValue` if encryption fails
41-55: Docs: Align verifiable function comments and returns description
- Similar “default params” issue as above.
- The Returns section is fine for the current type, but consider clarifying the array contents (Uint8Array and JSON string).
- The “Panics” section should be “Errors”.
-/// A function to encrypt a u64 value using BFV and default params and -/// generate circuit inputs for Greco +/// Encrypt a u64 value using BFV with explicit parameters and +/// generate circuit inputs for Greco @@ -/// Returns a `Result<Vec<JsValue>, JsValue>` containing the encrypted data, circuit inputs and any errors. +/// Returns a `Result<Vec<JsValue>, JsValue>` containing: +/// [0] Uint8Array ciphertext bytes +/// [1] circuit_inputs JSON string (for Greco proof generation) @@ -/// # Panics -/// -/// Panics if the data cannot be encrypted +/// # Errors +/// Returns an error as `JsValue` if encryption or circuit input generation failsAlso applies to: 56-59
crates/bfv-helpers/src/client.rs (2)
116-121: Avoid unwrap on constant modulus string (nit)
unwrap()can panic. Preferexpectwith context or precompute this modulus as a static to avoid recomputation and panics.- let zkp_modulus = BigInt::from_str_radix( + let zkp_modulus = BigInt::from_str_radix( "21888242871839275222246405745257275088548364400416034343698204186575808495617", 10, - ) - .unwrap(); + ).expect("valid BN254 scalar field modulus");Optional: move this into a
lazy_static/once_cell::sync::Lazyfor reuse.
35-41: API ergonomics: take public key as a slice to avoid unnecessary copiesBoth functions accept
public_key: Vec<u8>but immediately borrow it; taking&[u8]avoids an allocation at call sites that can pass a slice.-pub fn bfv_encrypt_u64( - data: u64, - public_key: Vec<u8>, +pub fn bfv_encrypt_u64( + data: u64, + public_key: &[u8], @@ - let pk = PublicKey::from_bytes(&public_key, ¶ms) + let pk = PublicKey::from_bytes(public_key, ¶ms)Apply the same change to
bfv_verifiable_encrypt_u64. Note: adjust callers accordingly (e.g., pass&public_key_bytes).Also applies to: 83-90
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (7)
Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasmis excluded by!**/*.wasmexamples/CRISP/apps/program/Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/program/methods/guest/Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/server/Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/wasm-crypto/Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
crates/bfv-helpers/Cargo.toml(1 hunks)crates/bfv-helpers/src/client.rs(2 hunks)crates/bfv-helpers/src/lib.rs(1 hunks)crates/bfv-helpers/src/util.rs(0 hunks)crates/evm-helpers/src/contracts.rs(1 hunks)crates/wasm/Cargo.toml(1 hunks)crates/wasm/README.md(1 hunks)crates/wasm/init_node.cjs(1 hunks)crates/wasm/init_node.js(1 hunks)crates/wasm/init_web.js(1 hunks)crates/wasm/package.json(2 hunks)crates/wasm/src/lib.rs(1 hunks)examples/CRISP/.env.example(1 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.js(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js(1 hunks)examples/CRISP/apps/client/package.json(1 hunks)examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts(3 hunks)examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx(1 hunks)examples/CRISP/apps/client/src/model/vote.model.ts(1 hunks)examples/CRISP/apps/client/src/utils/proof-encoding.ts(1 hunks)examples/CRISP/apps/client/src/utils/proofUtils.ts(0 hunks)examples/CRISP/apps/program/Cargo.toml(1 hunks)examples/CRISP/apps/server/src/server/mod.rs(1 hunks)examples/CRISP/apps/server/src/server/models.rs(1 hunks)examples/CRISP/apps/server/src/server/routes/voting.rs(3 hunks)examples/CRISP/apps/wasm-crypto/Cargo.toml(0 hunks)examples/CRISP/apps/wasm-crypto/README.md(0 hunks)examples/CRISP/apps/wasm-crypto/src/lib.rs(0 hunks)examples/CRISP/contracts/CRISPVerifier.sol(12 hunks)examples/CRISP/hardhat.config.ts(2 hunks)examples/CRISP/package.json(3 hunks)examples/CRISP/test/crisp.contracts.test.ts(1 hunks)examples/CRISP/tsconfig.json(1 hunks)packages/enclave-react/src/useEnclaveSDK.ts(4 hunks)packages/enclave-sdk/package.json(2 hunks)packages/enclave-sdk/src/enclave-sdk.ts(6 hunks)packages/enclave-sdk/src/greco.ts(1 hunks)packages/enclave-sdk/src/index.ts(2 hunks)packages/enclave-sdk/src/types.ts(2 hunks)packages/enclave-sdk/src/wasm.ts(0 hunks)packages/enclave-sdk/tests/sdk.test.ts(1 hunks)packages/enclave-sdk/tsup.config.js(1 hunks)packages/evm/package.json(1 hunks)templates/default/client/src/pages/WizardSDK.tsx(2 hunks)templates/default/server/index.ts(2 hunks)templates/default/tests/integration.spec.ts(3 hunks)
💤 Files with no reviewable changes (9)
- examples/CRISP/apps/wasm-crypto/README.md
- packages/enclave-sdk/src/wasm.ts
- crates/bfv-helpers/src/util.rs
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts
- examples/CRISP/apps/wasm-crypto/Cargo.toml
- examples/CRISP/apps/wasm-crypto/src/lib.rs
- examples/CRISP/apps/client/src/utils/proofUtils.ts
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.js
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- templates/default/client/src/pages/WizardSDK.tsx
- packages/evm/package.json
- crates/evm-helpers/src/contracts.rs
- examples/CRISP/apps/client/src/utils/proof-encoding.ts
- crates/bfv-helpers/Cargo.toml
- examples/CRISP/apps/server/src/server/models.rs
- crates/bfv-helpers/src/lib.rs
- packages/enclave-sdk/src/index.ts
- crates/wasm/init_node.js
- examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js
- examples/CRISP/tsconfig.json
- templates/default/server/index.ts
- examples/CRISP/apps/client/src/model/vote.model.ts
- examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx
- examples/CRISP/package.json
- packages/enclave-sdk/tsup.config.js
- crates/wasm/init_web.js
- examples/CRISP/hardhat.config.ts
- packages/enclave-sdk/src/greco.ts
- examples/CRISP/apps/server/src/server/routes/voting.rs
- templates/default/tests/integration.spec.ts
- crates/wasm/Cargo.toml
- crates/wasm/README.md
🧰 Additional context used
🧬 Code Graph Analysis (5)
packages/enclave-sdk/tests/sdk.test.ts (2)
examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (1)
sdk(18-28)packages/enclave-sdk/src/index.ts (1)
FheProtocol(42-42)
packages/enclave-sdk/src/enclave-sdk.ts (1)
packages/enclave-sdk/src/index.ts (1)
FheProtocol(42-42)
packages/enclave-react/src/useEnclaveSDK.ts (1)
packages/enclave-sdk/src/index.ts (1)
FheProtocol(42-42)
packages/enclave-sdk/src/types.ts (1)
packages/enclave-sdk/src/index.ts (1)
FheProtocol(42-42)
examples/CRISP/test/crisp.contracts.test.ts (2)
examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (1)
sdk(18-28)packages/enclave-sdk/src/index.ts (1)
FheProtocol(42-42)
🪛 Biome (2.1.2)
packages/enclave-sdk/src/enclave-sdk.ts
[error] 149-155: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 157-157: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 dotenv-linter (3.3.0)
examples/CRISP/.env.example
[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [UnorderedKey] The MNEMONIC key should go before the PRIVATE_KEY key
(UnorderedKey)
[warning] 3-3: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 Gitleaks (8.27.2)
examples/CRISP/test/crisp.contracts.test.ts
100-100: 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). (1)
- GitHub Check: e2e
🔇 Additional comments (14)
examples/CRISP/apps/server/src/server/mod.rs (1)
26-26: Good move: initialize logging early.Importing init_logger and initializing logging before building HttpServer ensures the Actix Logger middleware has a backend configured. Looks good.
examples/CRISP/apps/program/Cargo.toml (2)
32-36: RISC Zero version bumps look consistent with the ecosystem (LGTM).Upgrading to risc0-build/risc0-zkvm 2.2.0 and risc0-zkp 2.0.2 aligns with the ethereum-side crates pinned to tag v2.2.0. No concerns from the manifest perspective.
36-36: Confirm default-features = false on risc0-zkp is intentional for 2.2.0 stack.If you rely on any of the disabled defaults (e.g., accelerated backends), ensure they’re explicitly enabled in consumer crates. Otherwise, this pin is fine.
crates/wasm/init_node.cjs (1)
7-9: LGTM: CJS initializer is consistent and safeNo-op async initializer for Node is fine and matches the new API naming. Good to have a CommonJS entry for require consumers.
examples/CRISP/contracts/CRISPVerifier.sol (3)
480-496: Transcript round0 sizing and indices look consistent with pairing points inclusionGood: round0 allocates 3 + publicInputsSize + 12, then appends (publicInputsSize - PAIRING_POINTS_SIZE) public inputs and PAIRING_POINTS_SIZE pairing points contiguously before appending 12 w1..w3 limbs. No overlap.
1563-1588: Public inputs length check aligns with VK sizing and pairing point separationThe contract expects publicInputs.length == vk.publicInputsSize - PAIRING_POINTS_SIZE and passes the pairingPointObject via the proof. This is coherent with computePublicInputDelta and the transcript.
1600-1635: computePublicInputDelta correctly iterates main public inputs then pairing pointsThe numerator/denominator accumulators and offsets are advanced across both collections. Loop bounds use numPublicInputs - PAIRING_POINTS_SIZE and PAIRING_POINTS_SIZE, matching the VK convention.
examples/CRISP/test/crisp.contracts.test.ts (2)
175-186: No changes needed to CRISP ABI encoding in test
TheCRISPInputValidator.validateimplementation still expects itsdataargument to be ABI‐decoded as
(bytes semaphoreProof, bytes noirProof, bytes32[] noirPublicInputs, bytes vote)
and returns onlyvote. The existing test packs exactly those four fields withAbiCoder.encode(...), so it matches the on‐chain contract interface. You can leave the encoding inexamples/CRISP/test/crisp.contracts.test.tsunchanged.
175-183: Ignore this change –AbiCoder.defaultAbiCoder()exists in ethers v6
I’ve confirmed in v6.15.0 (both ESM and CJS builds) thatAbiCoderexposes a staticdefaultAbiCoder()method, which returns a singleton instance. The existing usage is valid and no update tonew AbiCoder()is needed.Likely an incorrect or invalid review comment.
examples/CRISP/apps/client/package.json (1)
20-20: Enclave SDK workspace dependency addition looks good.This aligns the client with the new SDK API surface introduced in this PR.
packages/enclave-sdk/package.json (1)
6-16: Dual ESM/CJS exports look correct.Having explicit import/require with main/module/types is a solid setup for consumers across bundlers.
packages/enclave-sdk/src/enclave-sdk.ts (2)
121-129: Ignore array suggestion formoduli– it’s a single bigint
TheProtocolParams.modulifield is declared as abigint(packages/enclave-sdk/src/types.ts:296), andBfvProtocolParams.BFV_NORMAL.moduliis likewise abigint. Passingthis.protocolParams.modulidirectly tobfv_encrypt_numberis correct. No change needed.Likely an incorrect or invalid review comment.
118-132: Confirmbigintusage for u64 parameters in wasm-bindgen
We weren’t able to locate the generated.d.tsfor@gnosis-guild/e3-wasmin this workspace, so please manually verify the TS signature ofbfv_encrypt_number. In particular:• Rust’s
u64→ JSbigintunder wasm-bindgen.
• EnsureplaintextModulusand each entry inmodulimatch thebiginttype in the d.ts (notnumber).If the definitions do require
bigint, coerce at the call site, for example:return bfv_encrypt_number( data, publicKey, this.protocolParams.degree, BigInt(this.protocolParams.plaintextModulus), this.protocolParams.moduli.map(BigInt), );– Check
node_modules/@gnosis-guild/e3-wasm/*.d.tsfor the exact parameter types and update accordingly.crates/bfv-helpers/src/client.rs (1)
130-178: Tests cover both encryption paths and decrypt round-trip — niceGood coverage validating both simple and verifiable encryption round-trips with the generated keypair; parameters are exercised via the provided set.
ctrlc03
force-pushed
the
feat/wasm-crypto-to-sdk
branch
from
a1cf438 to
0fc3bf0
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
examples/CRISP/apps/program/Cargo.toml (1)
5-5: Correct the excluded path in Cargo.toml
The settingexclude = ["methods/guests"]doesn’t match any directory (only
methods/guestexists). Please update it:- exclude = ["methods/guests"] + exclude = ["methods/guest"]• File:
examples/CRISP/apps/program/Cargo.toml(line 5)examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx (1)
33-59: Critical: Reassigning worker.onmessage introduces race conditions and promise leaks under concurrent callsEach call overwrites
worker.onmessage. If twoencryptVoterequests overlap, the earlier promise can be orphaned or both calls can resolve with the wrong response. Use a per-request correlation ID andaddEventListenerwith removal on match.Apply this diff to correlate replies and avoid clobbering:
@@ - worker.postMessage({ type: 'encrypt_vote', data: { voteId, publicKey } }) - worker.onmessage = async (event) => { - const { type, success, encryptedVote, error } = event.data - if (type === 'encrypt_vote') { - if (success) { - const { vote, proofData } = encryptedVote; - const { proof, publicInputs } = proofData - - resolve({ - vote: vote, - proof: proof, - public_inputs: publicInputs, - }) - } else { - showToast({ - type: 'danger', - message: error, - }) - handleGenericError('encryptVote', new Error(error)) - reject(new Error(error)) - } - setIsLoading(false) - } - } + const requestId = (globalThis.crypto?.randomUUID?.() ?? `${Date.now()}-${Math.random()}`).toString() + + const onMessage = (event: MessageEvent) => { + const { type, success, encryptedVote, error, requestId: rid } = event.data ?? {} + if (type !== 'encrypt_vote' || rid !== requestId) return + worker.removeEventListener('message', onMessage) + if (success) { + const { vote, proofData } = encryptedVote ?? {} + if (!proofData?.proof || !proofData?.publicInputs) { + const msg = 'Malformed worker response: missing proofData.proof or proofData.publicInputs' + showToast({ type: 'danger', message: msg }) + handleGenericError('encryptVote', new Error(msg)) + setIsLoading(false) + reject(new Error(msg)) + return + } + const { proof, publicInputs } = proofData + resolve({ + vote, + proof, + public_inputs: publicInputs, + }) + } else { + showToast({ type: 'danger', message: error }) + handleGenericError('encryptVote', new Error(error)) + reject(new Error(error)) + } + setIsLoading(false) + } + + const onError = (err: ErrorEvent) => { + worker.removeEventListener('message', onMessage) + setIsLoading(false) + const msg = err.message || 'Worker error during encrypt_vote' + showToast({ type: 'danger', message: msg }) + handleGenericError('encryptVote', err.error || new Error(msg)) + reject(err.error || new Error(msg)) + } + + worker.addEventListener('message', onMessage) + worker.addEventListener('error', onError, { once: true }) + + worker.postMessage({ type: 'encrypt_vote', data: { voteId, publicKey }, requestId })Additionally, update the worker (libs/wasm/pkg/crisp_worker.js) to echo
requestIdback in responses:// Inside the worker when posting the reply self.postMessage({ type: 'encrypt_vote', success: true, encryptedVote, requestId, // echo the id from the request }) // and in error cases: self.postMessage({ type: 'encrypt_vote', success: false, error: errorMessage, requestId, })crates/wasm/README.md (1)
3-3: Fix typos/grammar in README opening sentence“funcionality” → “functionality”, and “enable use to share” → “enable us to share”.
Apply this diff:
-Here we export wasm funcionality for consumption in typescript to enable use to share code between Rust and Typescript. +Here we export Wasm functionality for consumption in TypeScript to enable us to share code between Rust and TypeScript.
♻️ Duplicate comments (12)
examples/CRISP/.env.example (1)
3-4: Place INFURA_API_KEY before RPC_URL to match runtime usagePast review asked for INFURA_API_KEY to appear before RPC_URL to align with how hardhat config reads env vars. You added the key (good), but ordering is still off.
Apply this minimal reordering:
-RPC_URL="http://localhost:8545" -INFURA_API_KEY="" +INFURA_API_KEY="" +RPC_URL="http://localhost:8545"If you want to re-verify usages before changing, run:
#!/bin/bash set -euo pipefail echo "Searching JS/TS for env var usage..." rg -n -C2 -P '\bprocess\.env\.(PRIVATE_KEY|MNEMONIC|RPC_URL|INFURA_API_KEY)\b' -g '!**/node_modules/**' --type=ts --type=js || true echo echo "Searching Rust for env var usage..." rg -n -C2 -P 'std::env::var\(\s*"(PRIVATE_KEY|MNEMONIC|RPC_URL|INFURA_API_KEY)"\s*\)' --type=rust || true echo echo "Searching generic dotenv loads..." rg -n -C2 -P '\bdotenv\b' -g '!**/node_modules/**' --type=ts --type=js || trueexamples/CRISP/apps/program/Cargo.toml (1)
32-36: RISC Zero versions aligned to 2.2.x — good. Please verify workspace-wide consistency.These pins match the recommended alignment (risc0-build/zkvm 2.2.0 and zkp 2.0.2 with default-features = false). Approving here; suggest re-running the workspace audit to ensure no other manifests still pin older versions (to avoid resolver conflicts).
Run to audit all manifests:
#!/bin/bash set -euo pipefail echo "== RISC Zero versions across workspace ==" rg -n -C0 -g '!**/target/**' -P '^\s*(risc0-(build|zkvm|zkp)|risc0-.*ethereum)\s*=' echo echo "== Mismatches vs expected (build/zkvm = 2.2.0, zkp = 2.0.2) ==" rg -n -C0 -g '!**/target/**' -P '^\s*risc0-build\s*=\s*\{[^\}]*version\s*=\s*"(?!=2\.2\.0)' || true rg -n -C0 -g '!**/target/**' -P '^\s*risc0-zkvm\s*=\s*\{[^\}]*version\s*=\s*"(?!=2\.2\.0)' || true rg -n -C0 -g '!**/target/**' -P '^\s*risc0-zkp\s*=\s*\{[^\}]*version\s*=\s*"(?!=2\.0\.2)' || trueexamples/CRISP/test/crisp.contracts.test.ts (4)
20-21: Guard RPC_URL at runtime; non-null assertion won’t prevent undefined.Using the bang operator only silences TS. Provide a default or throw early to avoid passing undefined to SemaphoreEthers.
-const rpcUrl = process.env.RPC_URL! +const rpcUrl = process.env.RPC_URL ?? "http://localhost:8545" // If a default is undesirable, prefer an explicit guard: // if (!process.env.RPC_URL) throw new Error("RPC_URL is required for SemaphoreEthers");
26-38: Fix types: committee_public_key and tuple syntax.
- The server returns committee_public_key as a JSON array (number[]), not a Uint8Array.
- TypeScript tuple syntax should be [string, string], not string[2].
Apply:
interface State { @@ - committee_public_key: Uint8Array - emojis: string[2] + committee_public_key: number[] + emojis: [string, string] }Follow-up change at the usage site to convert to Uint8Array (see below).
92-101: Avoid committing secrets and align rpcUrl with env.Replace the hardcoded rpcUrl and privateKey with environment variables, or omit privateKey entirely if not required by this test (you sign via signer/contracts directly).
Apply:
- const sdk = EnclaveSDK.create({ - rpcUrl: "http://localhost:8545", + const sdk = EnclaveSDK.create({ + rpcUrl: process.env.RPC_URL ?? "http://localhost:8545", contracts: { enclave: enclaveAddress, ciphernodeRegistry: "0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0" }, chainId: 31337, protocol: FheProtocol.BFV, - privateKey: "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" + // Optional: only if SDK must sign txs within this test + // privateKey: process.env.TEST_PRIVATE_KEY as `0x${string}`, })Also ensure your CI/secrets tooling masks TEST_PRIVATE_KEY.
160-165: Convert server key to Uint8Array before passing to SDK.SDK expects a Uint8Array; the server returns number[]. Convert explicitly to prevent subtle runtime issues.
Apply:
- const stateJson = await state.json() as State + const stateJson = await state.json() as State @@ - const publicKey = stateJson.committee_public_key + const publicKey = new Uint8Array(stateJson.committee_public_key)examples/CRISP/contracts/CRISPVerifier.sol (1)
679-707: Likely proof field ordering mismatch: read w4 immediately after w1..w3.Current loader reads lookupReadCounts and lookupReadTags before w4, which is unusual and was previously flagged. Typical layout reads w1, w2, w3, w4, then lookup fields. If the prover encodes w4 right after w3, this ordering will corrupt reads.
Apply this reordering in loadProof, but only if it matches the prover’s actual encoding:
- // Lookup / Permutation Helper Commitments - p.lookupReadCounts = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); - boundary += 0x80; - p.lookupReadTags = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); - boundary += 0x80; - p.w4 = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); - boundary += 0x80; - p.lookupInverses = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); - boundary += 0x80; - p.zPerm = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); - boundary += 0x80; + // Free wire w4, then lookup/permutation helpers + p.w4 = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + boundary += 0x80; + p.lookupReadCounts = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + boundary += 0x80; + p.lookupReadTags = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + boundary += 0x80; + p.lookupInverses = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + boundary += 0x80; + p.zPerm = bytesToG1ProofPoint(proof[boundary:boundary + 0x80]); + boundary += 0x80;If your prover intentionally writes lookupReadCounts/tags before w4, then keep the loader as-is but ensure the transcript rounds (generateBetaAndGammaChallenges) are in sync with that same order.
crates/wasm/README.md (1)
21-23: Initializer naming consistency (init vs initializeWasm)Past feedback requested shifting examples to
initializeWasmto match the wrapper name used in both Node/Web shims. The current import/usage aliasinitworks (default import can be aliased), but usinginitializeWasmimproves clarity and discoverability.If you prefer consistency, update the import and call:
import initializeWasm from "@gnosis-guild/e3-wasm/init"; // ... await initializeWasm();packages/enclave-sdk/src/types.ts (2)
279-297: ProtocolParams.moduli should be an array (RNS primes) to match BFV conventions and future-proofingThe JSDoc example and BFV practice imply a collection of primes. Modeling this as bigint locks the API into single-prime configs and is hard to evolve without a breaking change later.
Apply:
export interface ProtocolParams { /** * The degree of the polynomial */ degree: number; /** * The plaintext modulus */ plaintextModulus: bigint; /** - * The moduli + * The moduli (RNS primes) */ - moduli: bigint; + moduli: readonly bigint[]; }Follow-up: update call sites to pass a single modulus element to the current WASM bindings (they take a single u64). See suggested changes in enclave-sdk.ts.
302-314: Update default BFV params to an array-based moduli to match the type and docsAlign defaults with the array type and the example in the comment.
export const BfvProtocolParams = { /** * Recommended parameters for BFV protocol * - Degree: 2048 * - Plaintext modulus: 1032193 - * - Moduli:0x3FFFFFFF000001 + * - Moduli: [0x3FFFFFFF000001] */ BFV_NORMAL: { degree: 2048, plaintextModulus: 1032193n, - moduli: 0x3FFFFFFF000001n, + moduli: [0x3FFFFFFF000001n], } as const satisfies ProtocolParams, }packages/enclave-sdk/tests/sdk.test.ts (1)
19-31: Remove committed private key and read from env; also make rpcUrl configurableAvoid committing secrets in tests. Use env vars and provide a sane default RPC URL to avoid transport init issues.
- rpcUrl: "", - privateKey: - "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80", + rpcUrl: process.env.RPC_URL ?? "http://127.0.0.1:8545", + // Prefer not committing keys; read from env if needed by the API. + privateKey: process.env.TEST_PRIVATE_KEY as `0x${string}`,If you want to omit the field when not set (preferred), declare above:
const TEST_PRIVATE_KEY = process.env.TEST_PRIVATE_KEY as `0x${string}` | undefined;…then spread it in:
const sdk = EnclaveSDK.create({ // ... ...(TEST_PRIVATE_KEY ? { privateKey: TEST_PRIVATE_KEY } : {}), protocol: FheProtocol.BFV, });packages/enclave-sdk/src/enclave-sdk.ts (1)
118-129: Pass a single modulus element to WASM bindings (if ProtocolParams.moduli becomes an array)When you retype ProtocolParams.moduli to an array, the WASM binding expects a single u64. Index into the first element.
case FheProtocol.BFV: - return bfv_encrypt_number( + return bfv_encrypt_number( data, publicKey, this.protocolParams.degree, this.protocolParams.plaintextModulus, - this.protocolParams.moduli, + this.protocolParams.moduli[0], );
🧹 Nitpick comments (12)
examples/CRISP/.env.example (1)
1-4: Optional: add trailing newline; keep quotes (MNEMONIC often contains spaces)
- Add a newline at EOF to silence the EndingBlankLine warning.
- The QuoteCharacter and UnorderedKey warnings can be safely ignored here. Quotes are practical for MNEMONIC phrases with spaces; enforcing alphabetical order provides little value for an example file.
Apply this tiny change:
INFURA_API_KEY="" +examples/CRISP/apps/program/Cargo.toml (2)
39-41: Pin git dependencies for reproducible builds.The FHE crates are sourced from Git without a tag or commit. Consider pinning to a tag or a specific commit to avoid non-deterministic builds when upstream changes.
Example diffs (use a real tag or commit SHA from the fhe.rs repo):
-fhe_rs = { package = "fhe", git = "https://github.com/gnosisguild/fhe.rs" } -fhe-traits = { git = "https://github.com/gnosisguild/fhe.rs" } -fhe-util = { git = "https://github.com/gnosisguild/fhe.rs" } +fhe_rs = { package = "fhe", git = "https://github.com/gnosisguild/fhe.rs", rev = "<commit-sha>" } +fhe-traits = { git = "https://github.com/gnosisguild/fhe.rs", rev = "<commit-sha>" } +fhe-util = { git = "https://github.com/gnosisguild/fhe.rs", rev = "<commit-sha>" }
43-45: Tokio “full” feature set may be heavier than needed.If you only need a subset (e.g., rt-multi-thread, macros, time, sync, fs, net), trimming features reduces compile time and binary size. If the example app intentionally uses broad Tokio surface, ignore.
Possible narrower feature set:
-tokio = { version = "=1.46.1", features = ["full"] } +tokio = { version = "=1.46.1", features = ["rt-multi-thread", "macros", "time", "sync", "fs", "net"] }examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx (3)
40-42: Switch to worker-supplied proofData looks good, but add a guard to avoid runtime crashes on malformed messagesIf
proofDatais undefined or missing fields, destructuring will throw at runtime. Add a defensive check before destructuring.Apply this diff:
- const { vote, proofData } = encryptedVote; - const { proof, publicInputs } = proofData + const { vote, proofData } = encryptedVote ?? {} + if (!proofData?.proof || !proofData?.publicInputs) { + const msg = 'Malformed worker response: missing proofData.proof or proofData.publicInputs' + showToast({ type: 'danger', message: msg }) + handleGenericError('encryptVote', new Error(msg)) + setIsLoading(false) + reject(new Error(msg)) + return + } + const { proof, publicInputs } = proofData
27-33: Optionally serialize calls to avoid UI flicker and overlapping requestsEven with correlation,
isLoadingis a single boolean, so concurrent requests can cause it to flip false while others are still in flight. If you don’t plan to support parallel encryptions, bail early when one is running.Apply this diff:
const encryptVote = async (voteId: bigint, publicKey: Uint8Array): Promise<EncryptedVote | undefined> => { - if (!worker) { + if (!worker) { console.error('WebAssembly worker not initialized') return } + if (isLoading) { + showToast({ type: 'warning', message: 'Encryption already in progress' }) + return + }If you do want true parallelism, consider tracking an in-flight counter instead of a boolean and set
isLoadingbased oncount > 0.
35-35: Micro-optimization: transfer the publicKey buffer to the worker to avoid cloningNot essential, but you can transfer a precise copy of the
Uint8Arraybuffer to the worker to reduce GC and copy costs.Example (replace the postMessage call accordingly):
// Make an exact-length copy to avoid transferring unused leading/trailing bytes. const keyBytes = publicKey.byteOffset === 0 && publicKey.byteLength === publicKey.buffer.byteLength ? publicKey : new Uint8Array(publicKey) // copies only the view's bytes worker.postMessage( { type: 'encrypt_vote', data: { voteId, publicKey: keyBytes }, requestId }, [keyBytes.buffer] // transfer ownership )examples/CRISP/tsconfig.json (1)
2-4: Confirm base config keeps strictness and Node/Hardhat types; otherwise set them here.You removed several safety flags locally. If the base tsconfig does not enforce them, enable:
- compilerOptions.strict: true
- compilerOptions.esModuleInterop: true (needed for default imports like dotenv and fs in tests)
- compilerOptions.types: ["node", "mocha", "chai", "hardhat"] (or project-specific)
This avoids runtime interop issues and missing globals in tests.
Apply if needed:
"extends": "@gnosis-guild/enclave-config/tsconfig.json", "include": ["tasks/**/*", "test/**/*", "deploy/**/*", "types/**/*"], "exclude": ["node_modules", "dist"], "compilerOptions": { + "strict": true, + "esModuleInterop": true, + "types": ["node", "mocha", "chai", "hardhat"], "module": "commonjs", "moduleResolution": "node", "incremental": true, "tsBuildInfoFile": "./dist/.tsbuildinfo", "resolveJsonModule": true, }examples/CRISP/test/crisp.contracts.test.ts (1)
176-184: Optional: use viem’s ABI encoder for consistency.You already use viem elsewhere in this file. Using a single ABI encoder avoids ethers/viem interop edge cases.
- const encodedInputs = AbiCoder.defaultAbiCoder().encode( - ["bytes", "bytes", "bytes32[]", "bytes"], - [ - semaphoreProof, - proof.proof, - proof.publicInputs, - encryptedVote - ] - ) + const params = parseAbiParameters("bytes, bytes, bytes32[], bytes") + const encodedInputs = encodeAbiParameters(params, [ + semaphoreProof, + proof.proof, + proof.publicInputs, + encryptedVote + ])If you keep ethers v6: ensure AbiCoder is from v6 (AbiCoder.defaultAbiCoder()) and inputs strictly match BytesLike/Hex types.
crates/wasm/init_node.js (1)
7-7: Optional: accept an unused initParams arg for signature parity with Web/typesKeeps the callable shape identical across environments and avoids potential TS typing drift if the .d.ts defines a parameter.
Apply this diff:
-export default async function initializeWasm() { +export default async function initializeWasm(_initParams) {crates/wasm/init_node.cjs (1)
7-7: Optional: accept an unused initParams arg for signature parity with Web/typesAligns callable shape across environments; future-proofs against .d.ts expectations.
Apply this diff:
-module.exports = async function initializeWasm() { +module.exports = async function initializeWasm(_initParams) {packages/enclave-sdk/src/enclave-sdk.ts (2)
79-91: Default protocol params selection is good; consider consistent SDKError usage for unsupported protocolsThe fallback to BFV defaults is handy. For consistency with the rest of the SDK, throw SDKError instead of Error for unsupported protocols.
switch (this.protocol) { case FheProtocol.BFV: this.protocolParams = BfvProtocolParams.BFV_NORMAL; break; default: - throw new Error("Protocol not supported") + throw new SDKError("Protocol not supported", "PROTOCOL_NOT_SUPPORTED"); }
129-131: Prefer SDKError over Error for consistencyUnify error shaping for easier handling upstream.
- default: - throw new Error("Protocol not supported") + default: + throw new SDKError("Protocol not supported", "PROTOCOL_NOT_SUPPORTED");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (7)
Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasmis excluded by!**/*.wasmexamples/CRISP/apps/program/Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/program/methods/guest/Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/server/Cargo.lockis excluded by!**/*.lockexamples/CRISP/apps/wasm-crypto/Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (49)
crates/bfv-helpers/Cargo.toml(1 hunks)crates/bfv-helpers/src/client.rs(2 hunks)crates/bfv-helpers/src/lib.rs(1 hunks)crates/bfv-helpers/src/util.rs(0 hunks)crates/evm-helpers/src/contracts.rs(1 hunks)crates/wasm/Cargo.toml(1 hunks)crates/wasm/README.md(1 hunks)crates/wasm/init_node.cjs(1 hunks)crates/wasm/init_node.js(1 hunks)crates/wasm/init_web.js(1 hunks)crates/wasm/package.json(2 hunks)crates/wasm/src/lib.rs(1 hunks)examples/CRISP/.env.example(1 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.js(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts(0 hunks)examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js(1 hunks)examples/CRISP/apps/client/package.json(1 hunks)examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts(3 hunks)examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx(1 hunks)examples/CRISP/apps/client/src/model/vote.model.ts(1 hunks)examples/CRISP/apps/client/src/utils/proof-encoding.ts(1 hunks)examples/CRISP/apps/client/src/utils/proofUtils.ts(0 hunks)examples/CRISP/apps/program/Cargo.toml(1 hunks)examples/CRISP/apps/server/src/server/mod.rs(1 hunks)examples/CRISP/apps/server/src/server/models.rs(1 hunks)examples/CRISP/apps/server/src/server/routes/voting.rs(3 hunks)examples/CRISP/apps/wasm-crypto/Cargo.toml(0 hunks)examples/CRISP/apps/wasm-crypto/README.md(0 hunks)examples/CRISP/apps/wasm-crypto/src/lib.rs(0 hunks)examples/CRISP/contracts/CRISPVerifier.sol(12 hunks)examples/CRISP/hardhat.config.ts(2 hunks)examples/CRISP/package.json(3 hunks)examples/CRISP/scripts/tasks/setup.sh(0 hunks)examples/CRISP/test/crisp.contracts.test.ts(1 hunks)examples/CRISP/tsconfig.json(1 hunks)packages/enclave-react/src/useEnclaveSDK.ts(4 hunks)packages/enclave-sdk/package.json(2 hunks)packages/enclave-sdk/src/enclave-sdk.ts(6 hunks)packages/enclave-sdk/src/greco.ts(1 hunks)packages/enclave-sdk/src/index.ts(2 hunks)packages/enclave-sdk/src/types.ts(2 hunks)packages/enclave-sdk/src/wasm.ts(0 hunks)packages/enclave-sdk/tests/sdk.test.ts(1 hunks)packages/enclave-sdk/tsup.config.js(1 hunks)packages/evm/package.json(1 hunks)templates/default/client/src/pages/WizardSDK.tsx(2 hunks)templates/default/server/index.ts(2 hunks)templates/default/tests/integration.spec.ts(3 hunks)
💤 Files with no reviewable changes (10)
- examples/CRISP/scripts/tasks/setup.sh
- examples/CRISP/apps/wasm-crypto/README.md
- crates/bfv-helpers/src/util.rs
- packages/enclave-sdk/src/wasm.ts
- examples/CRISP/apps/wasm-crypto/Cargo.toml
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.js
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts
- examples/CRISP/apps/client/src/utils/proofUtils.ts
- examples/CRISP/apps/wasm-crypto/src/lib.rs
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts
✅ Files skipped from review due to trivial changes (1)
- crates/wasm/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (26)
- examples/CRISP/apps/client/package.json
- crates/bfv-helpers/Cargo.toml
- crates/bfv-helpers/src/lib.rs
- packages/evm/package.json
- packages/enclave-sdk/tsup.config.js
- examples/CRISP/apps/server/src/server/mod.rs
- templates/default/tests/integration.spec.ts
- examples/CRISP/apps/client/src/model/vote.model.ts
- templates/default/client/src/pages/WizardSDK.tsx
- examples/CRISP/apps/client/src/utils/proof-encoding.ts
- templates/default/server/index.ts
- examples/CRISP/apps/server/src/server/models.rs
- examples/CRISP/apps/server/src/server/routes/voting.rs
- crates/evm-helpers/src/contracts.rs
- examples/CRISP/apps/client/src/hooks/voting/useVoteCasting.ts
- crates/wasm/init_web.js
- packages/enclave-react/src/useEnclaveSDK.ts
- examples/CRISP/hardhat.config.ts
- crates/wasm/package.json
- examples/CRISP/package.json
- packages/enclave-sdk/src/index.ts
- crates/wasm/src/lib.rs
- examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js
- packages/enclave-sdk/src/greco.ts
- crates/bfv-helpers/src/client.rs
- packages/enclave-sdk/package.json
🧰 Additional context used
🧬 Code Graph Analysis (6)
crates/wasm/init_node.cjs (2)
crates/wasm/init_node.js (1)
initializeWasm(7-9)crates/wasm/init_web.js (1)
initializeWasm(8-10)
crates/wasm/init_node.js (1)
crates/wasm/init_web.js (1)
initializeWasm(8-10)
examples/CRISP/test/crisp.contracts.test.ts (2)
examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (2)
sdk(18-28)data(15-15)packages/enclave-sdk/src/enclave-sdk.ts (1)
EnclaveSDK(42-474)
packages/enclave-sdk/tests/sdk.test.ts (2)
examples/CRISP/apps/client/libs/wasm/pkg/crisp_worker.js (1)
sdk(18-28)packages/enclave-sdk/src/enclave-sdk.ts (1)
EnclaveSDK(42-474)
packages/enclave-sdk/src/types.ts (1)
packages/enclave-sdk/src/index.ts (5)
SDKConfig(17-17)FheProtocol(42-42)ProtocolParams(37-37)VerifiableEncryptionResult(38-38)BfvProtocolParams(42-42)
packages/enclave-sdk/src/enclave-sdk.ts (5)
packages/enclave-sdk/src/types.ts (3)
ProtocolParams(284-297)BfvProtocolParams(302-314)VerifiableEncryptionResult(256-265)crates/wasm/init_node.js (1)
initializeWasm(7-9)crates/wasm/init_web.js (1)
initializeWasm(8-10)crates/wasm/src/lib.rs (2)
bfv_encrypt_number(28-38)bfv_verifiable_encrypt_number(59-74)packages/enclave-sdk/src/greco.ts (3)
CircuitInputs(23-37)defaultParams(81-97)generateProof(128-179)
🪛 dotenv-linter (3.3.0)
examples/CRISP/.env.example
[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [UnorderedKey] The MNEMONIC key should go before the PRIVATE_KEY key
(UnorderedKey)
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 4-4: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 4-4: [UnorderedKey] The INFURA_API_KEY key should go before the MNEMONIC key
(UnorderedKey)
🪛 Biome (2.1.2)
packages/enclave-sdk/src/enclave-sdk.ts
[error] 149-155: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 157-157: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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: integration_prebuild
- GitHub Check: test_contracts
- GitHub Check: build_enclave_cli
- GitHub Check: test_net
- GitHub Check: rust_unit
- GitHub Check: build_sdk
- GitHub Check: Build & Push Image
- GitHub Check: e2e
- GitHub Check: Build & Push Image
🔇 Additional comments (13)
examples/CRISP/apps/client/src/hooks/wasm/useWebAssembly.tsx (1)
27-34: EncryptedVote shape matches the type definitionI’ve verified that:
EncryptedVoteis defined as:
•vote: Uint8Array
•proof: Uint8Array
•public_inputs: string[]- The promise resolves an object
{ vote, proof, public_inputs }exactly matching that interface.- The worker posts
{ encryptedVote: { vote, proofData } }, whereproofDatacontains bothproofandpublicInputs, and the hook correctly destructures these intoproofandpublic_inputs.No changes are needed.
examples/CRISP/tsconfig.json (1)
6-10: CJS + node resolution is fine for Hardhat; keep resolveJsonModule for JSON circuit imports.The options look consistent with the test harness (fs/path, dynamic import("hardhat"), JSON circuit). No concerns if the base config supplies the remaining flags.
examples/CRISP/test/crisp.contracts.test.ts (1)
169-173: Good: wait for member registration tx to mine.This prevents flakiness when fetching members and generating the membership-dependent proof.
examples/CRISP/contracts/CRISPVerifier.sol (5)
389-411: Pairing point object integration looks coherent.Adding pairingPointObject into the proof and threading it through transcript and delta logic is consistent. No issues spotted here.
480-497: Transcript Round 0: appending pairing points after publicInputs is sensible; mind VK/publicInputsSize semantics.You treat vk.publicInputsSize as main inputs + pairing points, then append pairing points to round0 and exclude them from the calldata publicInputs. This matches the length check later. Looks fine.
1563-1571: PROOF_SIZE matches the new loader layout (456 field elements).Given the added 16 Fr and existing commitments/evaluations, the math checks out (14592 bytes / 32 = 456). Good.
1576-1588: Public inputs length check excludes pairing points — consistent with encode path.Your check against vk.publicInputsSize - PAIRING_POINTS_SIZE matches how you build round0 and compute deltas. No issues here.
1600-1635: Delta computation extends over pairing points; order must match prover’s encoding.The two-phase loop (first main public inputs, then pairingPointObject) is fine, provided the prover encodes pairing points at the end of its public input object in the same order. Confirm the writer aligns to prevent FS desync.
Do you want a script to locate the proof encoder and double-check the ordering assumptions across the repo?
crates/wasm/init_node.js (1)
7-9: LGTM: simple no-op initializer for Node is correctDefault async export and no-op body are appropriate for Node. Consumers can safely
awaitthis for parity with the Web initializer.crates/wasm/init_node.cjs (1)
7-9: LGTM: CJS no-op initializer complements the ESM wrapperMatches the ESM Node wrapper and the package.json export map pattern.
packages/enclave-sdk/src/types.ts (3)
7-13: Type-only imports are correct and avoid runtime bloatGood use of type-only imports for viem and aztec types; this keeps bundles slim.
253-265: Result shape for verifiable encryption looks goodThe VerifiableEncryptionResult interface is clear and aligns with the SDK API surface.
270-276: Protocol enum LGTMFheProtocol enum is a good base for expanding protocols later.
ctrlc03
force-pushed
the
feat/wasm-crypto-to-sdk
branch
from
cff0c33 to
71f2816
Compare
ctrlc03
force-pushed
the
feat/wasm-crypto-to-sdk
branch
from
71f2816 to
083245b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
package.json (1)
55-56: Unify build entry point: prefer build:ts and wire build/release to itAvoid parallel build entry points. Make "build" call "build:ts" and reuse it in "release".
- "build": "pnpm compile", + "build": "pnpm build:ts", @@ - "release": "pnpm build && changeset publish", + "release": "pnpm build:ts && changeset publish",examples/CRISP/package.json (2)
20-21: Simplify Hardhat test script to use configured tests pathLet Hardhat pick up tests from its configured directory; keeps paths DRY.
- "test:contracts": "hardhat test tests/crisp.contracts.test.ts --network localhost", + "test:contracts": "hardhat test --network localhost",
52-53: Drop duplicate Playwright package@playwright/test includes the CLI; the separate "playwright" dep is redundant.
- "@playwright/test": "1.52.0", - "playwright": "1.52.0", + "@playwright/test": "1.52.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/ci.yml(3 hunks)examples/CRISP/package.json(3 hunks)package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
⏰ 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: build_sdk
- GitHub Check: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: test_contracts
- GitHub Check: test_net
- GitHub Check: rust_unit
- GitHub Check: Build & Push Image
🔇 Additional comments (1)
package.json (1)
62-69: All overridden toolchain versions are valid Confirmed typescript@5.8.3, tsup@8.5.0, viem@2.30.6, @types/node@22.7.5, and undici-types@6.19.8 exist on npm.
0xjei
left a comment
0xjei
left a comment
There was a problem hiding this comment.
tACK - works on my end, feel free to merge once the CI green
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
package.json (1)
13-13: Compile now correctly sequences wasm before SDK (resolves previous blocker).Routing compile through build:ts ensures wasm builds before sdk, addressing the prior review.
🧹 Nitpick comments (2)
package.json (2)
54-54: Use build:ts in release to avoid unnecessary Rust/circuit builds during publish.This prevents release jobs from requiring Cargo/Noir toolchains when only JS/wasm packages are being published.
Apply:
- "release": "pnpm build && changeset publish", + "release": "pnpm build:ts && changeset publish",
45-45: Path style nit: make cd consistent with the rest of the file.Most scripts use
cd crates/...(no./). Align for consistency.- "wasm:build": "cd ./crates/wasm && pnpm build", + "wasm:build": "cd crates/wasm && pnpm build",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/ci.yml(3 hunks)examples/CRISP/package.json(3 hunks)package.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/CRISP/package.json
- .github/workflows/ci.yml
⏰ 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_e3_support_risc0
- GitHub Check: build_sdk
- GitHub Check: rust_unit
- GitHub Check: test_contracts
- GitHub Check: test_net
- GitHub Check: integration_prebuild
- GitHub Check: Build & Push Image
- GitHub Check: Build & Push Image
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
packages/enclave-sdk/tests/sdk.test.ts (3)
26-28: Remove hardcoded private key and read RPC URL/PK from env (or omit PK entirely).Committed secrets are a security risk, even if they’re Anvil defaults. Prefer env vars; if a wallet isn’t needed for these tests, drop the privateKey field.
- rpcUrl: "", - privateKey: - "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80", + rpcUrl: process.env.RPC_URL ?? "http://localhost:8545", + // Avoid committing keys; use env if/when a wallet is required. + ...(process.env.TEST_PRIVATE_KEY + ? { privateKey: process.env.TEST_PRIVATE_KEY as `0x${string}` } + : {}),
32-40: Fix typo and switch to Vitest matchers; avoid brittle exact length assertion.The current Chai-style assertions will fail under Vitest. Also, asserting an exact ciphertext size is fragile given parameter changes.
- it("should encrypt a number without crashing in a node environent", async () => { + it("should encrypt a number without crashing in a node environment", async () => { const buffer = await fs.readFile( path.resolve(__dirname, "./fixtures/pubkey.bin"), ); const value = await sdk.encryptNumber(10n, Uint8Array.from(buffer)); - expect(value).to.be.an.instanceof(Uint8Array); - expect(value.length).to.equal(27_674); + expect(value).toBeInstanceOf(Uint8Array); + // Be resilient to parameter updates: + expect(value.length).toBeGreaterThan(0); // TODO: test the encryption is correct });
41-51: Second test: fix typo and matcher style; relax expectations; reduce extreme timeout.Use Vitest matchers and a sane timeout; validate shape rather than constructor of generic Object.
- it("should encrypt a number and generate a proof without crashing in a node environent", async () => { + it("should encrypt a number and generate a proof without crashing in a node environment", async () => { const buffer = await fs.readFile( path.resolve(__dirname, "./fixtures/pubkey.bin"), ); const value = await sdk.encryptNumberAndGenProof(1n, Uint8Array.from(buffer), demoCircuit as unknown as CompiledCircuit); - expect(value).to.be.an.instanceof(Object); - expect(value.encryptedVote).to.be.an.instanceof(Uint8Array); - expect(value.proof).to.be.an.instanceOf(Object) - }, 9999999); + expect(value).toBeDefined(); + expect(value.encryptedVote).toBeInstanceOf(Uint8Array); + expect(value.proof).toBeDefined(); + }, 120_000);packages/enclave-sdk/src/enclave-sdk.ts (1)
148-167: Wrap BFV case in a block (noSwitchDeclarations), guard JSON.parse, and standardize errors.Declarations directly in a switch clause violate lints and can leak scope; also add robust parsing and consistent SDKError usage.
- case FheProtocol.BFV: - const [encryptedVote, circuitInputs] = bfv_verifiable_encrypt_number( - data, - publicKey, - this.protocolParams.degree, - this.protocolParams.plaintextModulus, - this.protocolParams.moduli, - ); - - const inputs = JSON.parse(circuitInputs) as CircuitInputs; - // inputs.params = defaultParams; - const proof = await generateProof(inputs, circuit); - - return { - encryptedVote, - proof, - }; + case FheProtocol.BFV: { + const [encryptedVote, circuitInputs] = bfv_verifiable_encrypt_number( + data, + publicKey, + this.protocolParams.degree, + this.protocolParams.plaintextModulus, + this.protocolParams.moduli, + ) as [Uint8Array, string]; + let inputs: CircuitInputs; + try { + inputs = JSON.parse(circuitInputs) as CircuitInputs; + } catch (e) { + throw new SDKError(`Invalid circuit inputs from WASM: ${String(e)}`, "WASM_OUTPUT_PARSE_ERROR"); + } + const proof = await generateProof(inputs, circuit); + return { encryptedVote, proof }; + } default: - throw new Error("Protocol not supported") + throw new SDKError("Protocol not supported", "PROTOCOL_NOT_SUPPORTED")examples/CRISP/tests/crisp.contracts.test.ts (2)
37-38: Fix types: committee_public_key is hex; tuple type is invalid.
- JSON cannot carry Uint8Array; the API returns a 0x-hex string.
string[2]is not a valid TS tuple type; use[string, string].- committee_public_key: Uint8Array - emojis: string[2] + // 0x-prefixed hex string from the API + committee_public_key: `0x${string}` + emojis: [string, string]
163-174: Decode committee_public_key before passing to the SDK.SDK expects bytes; you’re passing a hex string through. Decode first, then call.
- const publicKey = stateJson.committee_public_key + const publicKeyHex = stateJson.committee_public_key as `0x${string}` + const publicKeyBytes = hexToBytes(publicKeyHex) @@ - const { proof, encryptedVote } = await sdk.encryptNumberAndGenProof(0n, publicKey, circuit) + const { proof, encryptedVote } = await sdk.encryptNumberAndGenProof(0n, publicKeyBytes, circuit)
🧹 Nitpick comments (16)
packages/evm/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json (1)
588-591: Provide a compatibility alias in the SDK to smooth the rename.If external apps import RequestParams from your generated types or SDK, add a TS alias (export type RequestParams = E3RequestParams) and deprecate it in release notes to avoid immediate breakage. Also bump the appropriate semver and note the rename in CHANGELOG.
packages/enclave-sdk/src/enclave-sdk.ts (6)
84-91: Use SDKError for unsupported protocol to keep error handling consistent.Constructor currently throws a generic Error; prefer SDKError with a code.
switch (this.protocol) { case FheProtocol.BFV: this.protocolParams = BfvProtocolParams.BFV_NORMAL; break; default: - throw new Error("Protocol not supported") + throw new SDKError("Protocol not supported", "PROTOCOL_NOT_SUPPORTED"); }
118-131: Standardize error type in encryptNumber default branch.Return SDKError instead of Error for consistency with the rest of the SDK.
default: - throw new Error("Protocol not supported") + throw new SDKError("Protocol not supported", "PROTOCOL_NOT_SUPPORTED")
97-101: Clarify/remove TODO: initialize() is used by request/activate/publish methods.The TODO claims redundancy, but multiple methods rely on initialized state. Either remove the comment or refactor callers to avoid it.
- // TODO: Delete this it is redundant
184-184: Remove stray console.log from library code.Avoid noisy stdout in published SDKs; gate logs behind a debug flag if needed.
- console.log(">>> REQUEST");
348-356: Method is marked async but doesn’t await; make it sync or await the promise.Current code returns immediately; either remove async or await the call.
Option A (make sync):
- public async startEventPolling(): Promise<void> { - void this.eventListener.startPolling(); - } + public startEventPolling(): void { + void this.eventListener.startPolling(); + }Option B (actually await):
- public async startEventPolling(): Promise<void> { - void this.eventListener.startPolling(); - } + public async startEventPolling(): Promise<void> { + await this.eventListener.startPolling(); + }
118-129: Optional: validate inputs before calling WASM (range and key checks).Prevent subtle WASM failures by validating data ∈ [0, 2^64) and non-empty publicKey.
public async encryptNumber(data: bigint, publicKey: Uint8Array): Promise<Uint8Array> { await initializeWasm(); + if (data < 0n || data >= (1n << 64n)) { + throw new SDKError("data must be a u64", "INVALID_ARGUMENT"); + } + if (!publicKey?.length) { + throw new SDKError("publicKey is required", "INVALID_ARGUMENT"); + } switch (this.protocol) {Apply similar checks in encryptNumberAndGenProof before the switch.
Also applies to: 149-161
package.json (1)
13-13: Avoid double-building EVM during compile.compile runs build:ts (which already runs evm:build) and then ciphernode:build, which has a preciphernode:build that runs evm:build again. Consider skipping the pre hook when compile is used to reduce CI time.
Example approaches (outside range):
- Remove preciphernode:build or gate it via an env flag.
- Split ciphernode:build into “evmless” and full variants and call the former from compile.
examples/CRISP/package.json (2)
43-45: Pin GitHub dependency to a commit for reproducible installs.github:hmzakhalid/semaphore-noir#dist-core tracks a branch; prefer a commit SHA to stabilize CI.
Example:
- "@semaphore-protocol/core": "github:hmzakhalid/semaphore-noir#dist-core", + "@semaphore-protocol/core": "github:hmzakhalid/semaphore-noir#<commit-sha>",
51-51: Align @types/node with runtime Node in CI.CI uses Node 20 for CRISP e2e; types 22 may expose APIs not present at runtime. Consider downgrading to Node 20 types or bump CRISP e2e to Node 22 for consistency.
examples/CRISP/tests/crisp.contracts.test.ts (6)
94-94: Use the rpcUrl variable already defined from env.Keeps config DRY and enables switching RPC via ENV.
- rpcUrl: "http://localhost:8545", + rpcUrl,
101-101: Avoid hard-coded private key in tests; allow override via env.This was flagged by the scanner. Prefer env with a sensible default for local Anvil.
- privateKey: "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" + privateKey: process.env.TEST_PRIVATE_KEY ?? "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"
120-120: Remove unnecessary async from describe.Mocha doesn’t await async describe callbacks; keep it sync.
- describe("voting", async () => { + describe("voting", () => {
75-81: Handle empty group edge case before generating a Semaphore proof.Avoid opaque failures when the group hasn’t indexed members yet.
const groupMembers = await ethersRef.getGroupMembers(groupId.toString()); + if (groupMembers.length === 0) { + throw new Error("Semaphore group has no members yet; cannot generate proof"); + } const group = new Group(groupMembers);
122-141: Check HTTP response status before parsing JSON (round/state).Prevents misleading test failures on non-2xx responses.
- const data = await fetch(stateEndpoint) + const data = await fetch(stateEndpoint) + if (!data.ok) throw new Error(`GET ${stateEndpoint} failed: ${data.status} ${data.statusText}`) const json = await data.json() as Round @@ - const state = await fetch(`${server}/state/lite`, { + const state = await fetch(`${server}/state/lite`, { method: "POST", body: JSON.stringify({ round_id: roundId }), headers: { "Content-Type": "application/json" } }) + if (!state.ok) throw new Error(`POST ${server}/state/lite failed: ${state.status} ${state.statusText}`) const stateJson = await state.json() as State
145-160: Do the same HTTP status checks in the voting flow.Consistent error handling aids debugging flaky CI.
- const data = await fetch(stateEndpoint) + const data = await fetch(stateEndpoint) + if (!data.ok) throw new Error(`GET ${stateEndpoint} failed: ${data.status} ${data.statusText}`) const json = await data.json() as Round @@ - const state = await fetch(`${server}/state/lite`, { + const state = await fetch(`${server}/state/lite`, { method: "POST", body: JSON.stringify({ round_id: roundId }), headers: { "Content-Type": "application/json" } }) + if (!state.ok) throw new Error(`POST ${server}/state/lite failed: ${state.status} ${state.statusText}`) const stateJson = await state.json() as State
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.lockexamples/CRISP/Cargo.lockis excluded by!**/*.lockexamples/CRISP/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasmis excluded by!**/*.wasmexamples/CRISP/wasm-crypto/Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yamltemplates/default/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (60)
.github/workflows/ci.yml(3 hunks)crates/bfv-helpers/Cargo.toml(1 hunks)crates/bfv-helpers/src/client.rs(2 hunks)crates/bfv-helpers/src/lib.rs(1 hunks)crates/bfv-helpers/src/util.rs(0 hunks)crates/program-server/src/lib.rs(0 hunks)crates/support-scripts/dev/Cargo.toml(1 hunks)crates/support/app/src/main.rs(0 hunks)crates/wasm/Cargo.toml(1 hunks)crates/wasm/README.md(1 hunks)crates/wasm/init_node.cjs(1 hunks)crates/wasm/init_node.js(1 hunks)crates/wasm/init_web.js(1 hunks)crates/wasm/package.json(2 hunks)crates/wasm/src/lib.rs(1 hunks)examples/CRISP/Cargo.toml(0 hunks)examples/CRISP/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts(0 hunks)examples/CRISP/client/libs/wasm/pkg/crisp_wasm_crypto.js(0 hunks)examples/CRISP/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts(0 hunks)examples/CRISP/client/libs/wasm/pkg/crisp_worker.js(1 hunks)examples/CRISP/client/package.json(2 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/utils/proof-encoding.ts(1 hunks)examples/CRISP/client/src/utils/proofUtils.ts(0 hunks)examples/CRISP/client/vite.config.ts(1 hunks)examples/CRISP/contracts/CRISPVerifier.sol(1 hunks)examples/CRISP/hardhat.config.ts(1 hunks)examples/CRISP/package.json(3 hunks)examples/CRISP/playwright.config.ts(2 hunks)examples/CRISP/program/Cargo.toml(1 hunks)examples/CRISP/scripts/dev.sh(1 hunks)examples/CRISP/scripts/setup.sh(0 hunks)examples/CRISP/scripts/test_e2e.sh(1 hunks)examples/CRISP/server/Dockerfile(0 hunks)examples/CRISP/server/src/server/mod.rs(1 hunks)examples/CRISP/server/src/server/models.rs(1 hunks)examples/CRISP/server/src/server/routes/voting.rs(1 hunks)examples/CRISP/test/wallet-setup/basic.setup.ts(1 hunks)examples/CRISP/tests/crisp.contracts.test.ts(1 hunks)examples/CRISP/tsconfig.json(1 hunks)examples/CRISP/wasm-crypto/Cargo.toml(0 hunks)examples/CRISP/wasm-crypto/README.md(0 hunks)examples/CRISP/wasm-crypto/src/lib.rs(0 hunks)package.json(3 hunks)packages/enclave-react/src/useEnclaveSDK.ts(4 hunks)packages/enclave-sdk/package.json(2 hunks)packages/enclave-sdk/src/enclave-sdk.ts(6 hunks)packages/enclave-sdk/src/greco.ts(1 hunks)packages/enclave-sdk/src/index.ts(2 hunks)packages/enclave-sdk/src/types.ts(2 hunks)packages/enclave-sdk/src/wasm.ts(0 hunks)packages/enclave-sdk/tests/sdk.test.ts(1 hunks)packages/enclave-sdk/tsup.config.js(1 hunks)packages/evm/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json(1 hunks)packages/evm/package.json(1 hunks)templates/default/client/src/pages/WizardSDK.tsx(2 hunks)templates/default/program/Cargo.toml(1 hunks)templates/default/server/index.ts(2 hunks)templates/default/tests/integration.spec.ts(3 hunks)
💤 Files with no reviewable changes (14)
- crates/support/app/src/main.rs
- examples/CRISP/wasm-crypto/README.md
- examples/CRISP/server/Dockerfile
- crates/program-server/src/lib.rs
- packages/enclave-sdk/src/wasm.ts
- examples/CRISP/client/src/utils/proofUtils.ts
- examples/CRISP/wasm-crypto/src/lib.rs
- examples/CRISP/wasm-crypto/Cargo.toml
- crates/bfv-helpers/src/util.rs
- examples/CRISP/Cargo.toml
- examples/CRISP/client/libs/wasm/pkg/crisp_wasm_crypto.js
- examples/CRISP/client/libs/wasm/pkg/crisp_wasm_crypto.d.ts
- examples/CRISP/scripts/setup.sh
- examples/CRISP/client/libs/wasm/pkg/crisp_wasm_crypto_bg.wasm.d.ts
✅ Files skipped from review due to trivial changes (2)
- examples/CRISP/server/src/server/routes/voting.rs
- crates/bfv-helpers/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (35)
- examples/CRISP/test/wallet-setup/basic.setup.ts
- examples/CRISP/client/package.json
- packages/enclave-sdk/package.json
- examples/CRISP/client/src/hooks/voting/useVoteCasting.ts
- crates/wasm/init_node.cjs
- examples/CRISP/hardhat.config.ts
- crates/wasm/Cargo.toml
- examples/CRISP/contracts/CRISPVerifier.sol
- templates/default/client/src/pages/WizardSDK.tsx
- examples/CRISP/tsconfig.json
- templates/default/server/index.ts
- examples/CRISP/client/vite.config.ts
- crates/bfv-helpers/src/client.rs
- examples/CRISP/client/src/hooks/wasm/useWebAssembly.tsx
- examples/CRISP/server/src/server/models.rs
- examples/CRISP/playwright.config.ts
- crates/wasm/init_web.js
- crates/wasm/README.md
- packages/evm/package.json
- crates/support-scripts/dev/Cargo.toml
- examples/CRISP/server/src/server/mod.rs
- templates/default/program/Cargo.toml
- packages/enclave-sdk/src/types.ts
- templates/default/tests/integration.spec.ts
- packages/enclave-react/src/useEnclaveSDK.ts
- crates/bfv-helpers/Cargo.toml
- examples/CRISP/scripts/test_e2e.sh
- examples/CRISP/program/Cargo.toml
- examples/CRISP/client/libs/wasm/pkg/crisp_worker.js
- packages/enclave-sdk/tsup.config.js
- packages/enclave-sdk/src/index.ts
- examples/CRISP/client/src/utils/proof-encoding.ts
- crates/wasm/package.json
- crates/wasm/src/lib.rs
- packages/enclave-sdk/src/greco.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-22T02:10:34.864Z
Learnt from: ryardley
PR: gnosisguild/enclave#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:
packages/enclave-sdk/tests/sdk.test.ts
🧬 Code graph analysis (4)
examples/CRISP/tests/crisp.contracts.test.ts (1)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (2)
sdk(18-30)data(15-15)
packages/enclave-sdk/tests/sdk.test.ts (2)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (1)
sdk(18-30)packages/enclave-sdk/src/enclave-sdk.ts (1)
EnclaveSDK(42-474)
packages/enclave-sdk/src/enclave-sdk.ts (5)
packages/enclave-sdk/src/types.ts (3)
ProtocolParams(284-297)BfvProtocolParams(302-314)VerifiableEncryptionResult(256-265)crates/wasm/init_node.js (1)
initializeWasm(7-9)crates/wasm/init_web.js (1)
initializeWasm(8-10)crates/wasm/src/lib.rs (2)
bfv_encrypt_number(28-38)bfv_verifiable_encrypt_number(59-74)packages/enclave-sdk/src/greco.ts (2)
CircuitInputs(23-37)generateProof(128-179)
crates/wasm/init_node.js (1)
crates/wasm/init_web.js (1)
initializeWasm(8-10)
🪛 Gitleaks (8.27.2)
examples/CRISP/tests/crisp.contracts.test.ts
101-101: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Biome (2.1.2)
packages/enclave-sdk/src/enclave-sdk.ts
[error] 149-155: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 157-157: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 159-159: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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: rust_unit
- GitHub Check: build_sdk
- GitHub Check: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: test_contracts
- GitHub Check: test_net
- GitHub Check: Build & Push Image
🔇 Additional comments (6)
packages/evm/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json (1)
588-591: All references updated — no issues remain. Solidity defines only E3RequestParams, TS/SDK has no lingering RequestParams, and the ABI internalType aligns..github/workflows/ci.yml (1)
376-378: wasm-pack step alone is insufficient if you keep local builds.If you proceed with Option B above, keep this step but also install Rust; if you adopt Option A (artifacts), remove this step to save time.
package.json (2)
45-45: LGTM: explicit wasm build hook.Adding a top-level wasm:build clarifies the build graph and unblocks downstream SDK builds.
54-56: LGTM: centralized TypeScript build orchestration.release delegating to build; build:ts sequencing evm → wasm → sdk → react looks correct.
examples/CRISP/package.json (1)
20-20: LGTM: fixed contracts test path (tests/).This aligns with the configured tests folder and avoids HH test discovery issues.
examples/CRISP/tests/crisp.contracts.test.ts (1)
48-63: encodeSemaphoreProof implementation looks correct.ABI layout and conversions (bytesToHex → encode → hexToBytes) are consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
examples/CRISP/package.json (3)
20-20: Good fix: contract test pathPoints to ./tests and specific file; matches prior Hardhat path config. Optionally rely on configured tests dir to run all tests:
- "test:contracts": "hardhat test tests/crisp.contracts.test.ts --network localhost", + "test:contracts": "hardhat test --network localhost",
27-27: SDK dependency via workspace looks correctUsing
"@gnosis-guild/enclave-sdk": "workspace:*"is appropriate for the monorepo. Ensure the WASM package is built before any SDK-dependent e2e that run here.Consider adding a pretest at the workspace root or here if CRISP e2e import SDK WASM at runtime:
"pretest": "pnpm -w wasm:build"
33-66: Toolchain compatibility confirmedNode v24.3.0, Hardhat v2.26.3, Playwright v1.52.0 verified against package.json. DevDeps are consistent.
Optional: add
"postinstall": "playwright install --with-deps"to examples/CRISP/package.json to ensure Playwright browsers are installed in CI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/ci.yml(3 hunks)examples/CRISP/package.json(3 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
⏰ 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_e3_support_dev
- GitHub Check: build_enclave_cli
- GitHub Check: integration_prebuild
- GitHub Check: test_contracts
- GitHub Check: rust_unit
- GitHub Check: test_net
- GitHub Check: Build & Push Image
🔇 Additional comments (2)
package.json (1)
54-54: LGTM: release scriptNo issues; trailing comma is correct with the added entry below.
examples/CRISP/package.json (1)
6-6: Keep “type”: “module” — Hardhat config is ESM-ready
hardhat.config.ts usesexport default config, so the project supports ESM as configured.
2 participants
fix #585
Note that Vercel deployment will fail as it requires published updated packages as vercel does not rebuild the monorepo
Summary by CodeRabbit
New Features
Breaking Changes
Documentation
Tests
Chores