chore: update Noir to latest version in CRISP [skip-line-limit]#1026
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
WalkthroughThis PR upgrades Noir toolchain and dependencies (noir_js from beta.3 to beta.15, bb.js to 3.0.0-nightly.20251104), refactors the Greco interface structure with nested parameters and additional circuit inputs, updates CRISP contract deployments to use library linking for HonkVerifier, and replaces proof option flags from keccak to keccakZK. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HonkVerifier
participant ZKTranscriptLib
Note over Client,ZKTranscriptLib: Library Linking Pattern (New)
Client->>ZKTranscriptLib: Deploy ZKTranscriptLib
activate ZKTranscriptLib
ZKTranscriptLib-->>Client: Returns library address
deactivate ZKTranscriptLib
Client->>HonkVerifier: Create factory with linked library
Note right of HonkVerifier: constructorFactory with<br/>libraries mapping
Client->>HonkVerifier: Deploy HonkVerifier (linked)
activate HonkVerifier
HonkVerifier-->>Client: Deployed with library linked
deactivate HonkVerifier
sequenceDiagram
participant SDK as EnclaveSDK
participant Validator
participant Encryption
Note over SDK,Encryption: Protocol Validation & Encryption Flow (Updated)
SDK->>Validator: Validate protocol in constructor
alt Protocol matches allowed values
Validator-->>SDK: Validation passes
else Protocol invalid
Validator-->>SDK: Throw INVALID_PROTOCOL error
end
SDK->>Encryption: encryptNumber(data)
Note right of Encryption: Direct BFV path<br/>(no switch statement)
Encryption->>Encryption: bfv_encrypt_number()
Encryption-->>SDK: Return encrypted result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cedoor
changed the title
cedoor
requested review from
0xjei and
ctrlc03
and removed request for
ctrlc03
|
Should we get rid of the functions to generate/verify GRECO-only proofs in the Enclave SDK? That is currently generating GRECO proofs without application logic (no checks on plaintext), which is not the way apps will build their circuits. So I'm not sure when that functionality can actually be useful for devs. Am I missing anything? Also, the circuit which was used to generate the .json artifact for the sdk is no longer in our repo, as we built an actual CRISP circuit. Something we could do is to provide functions to wrap noir/bb without passing any specific inputs and without using any specific circuit artifact, which will be passed by devs based on their circuits. |
cedoor
force-pushed
the
chore/latest-noir
branch
from
8eae070 to
4776727
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
examples/CRISP/server/.env.example (1)
16-19: Reorder environment variable keys alphabetically.The dotenv-linter tool flags that environment variable keys should be ordered alphabetically. E3_PROGRAM_ADDRESS should precede ENCLAVE_ADDRESS (E3 < EN). Consider reordering the block for consistency with linting standards.
Apply this diff to fix the key ordering:
# Based on Default Hardhat Deployments (Only for testing) -ENCLAVE_ADDRESS="0xA51c1fc2f0D1a1b8494Ed1FE312d7C3a78Ed91C0" -CIPHERNODE_REGISTRY_ADDRESS="0x610178dA211FEF7D417bC0e6FeD39F05609AD788" E3_PROGRAM_ADDRESS="0x67d269191c92Caf3cD7723F116c85e6E9bf55933" # CRISPProgram Contract Address +ENCLAVE_ADDRESS="0xA51c1fc2f0D1a1b8494Ed1FE312d7C3a78Ed91C0" +CIPHERNODE_REGISTRY_ADDRESS="0x610178dA211FEF7D417bC0e6FeD39F05609AD788" FEE_TOKEN_ADDRESS="0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0"examples/CRISP/packages/crisp-contracts/hardhat.config.ts (1)
162-165: Confirm optimizer runs=100 is the intended deployment/runtime trade‑offLowering
optimizer.runsto 100 will minimize bytecode size (cheaper deployment) at the cost of somewhat higher runtime gas; if CRISP is expected to handle heavy on‑chain load, double‑check this setting matches your gas/size priorities.examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (1)
111-127: Library‑linked HonkVerifier deployment pattern looks good; just confirm the FQNThe pattern of:
- deploying
ZKTranscriptLib,- passing its address via
librarieswhen creating theHonkVerifierfactory,- then deploying
HonkVerifier,matches how you’ve wired things in the deploy script and should work well. Please just double‑check that the key
"project/contracts/CRISPVerifier.sol:ZKTranscriptLib"matches the fully‑qualified library name in the compiled artifact’slinkReferences; if it ever changes, this test and the deploy script will both need updating.examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (3)
39-59: ZKTranscriptLib + HonkVerifier deployment is sound; verify FQN and consider waiting for deploymentThe flow of deploying
ZKTranscriptLib, wiring it into theHonkVerifierfactory vialibraries, deployingHonkVerifier, and persisting its address viastoreDeploymentArgslooks correct and matches the test setup.Two small follow‑ups to consider:
- Make sure
"project/contracts/CRISPVerifier.sol:ZKTranscriptLib"exactly matches the library’s fully‑qualified name in the CRISPVerifier artifact; a mismatch will break deployment in a non‑obvious way.- For consistency with the other verifiers, you might add
await honkVerifier.waitForDeployment();before reading the address, even though Hardhat’s auto‑mining usually makes this redundant.
60-89: CRISPProgram vs MockCRISPProgram switch and stored constructorArgs look reasonableUsing
useMockInputValidatorto switch betweenMockCRISPProgramandCRISPProgram, and then storing a structuredconstructorArgsobject withenclave,verifierAddress,honkVerifierAddress, andimageIdis a clean way to keep deployments introspectable.One minor nit:
Boolean(process.env.USE_MOCK_INPUT_VALIDATOR)treats any non‑empty string (including"false") astrue. If you expect values like"true"/"false"or"1"/"0", a more explicit check (e.g.process.env.USE_MOCK_INPUT_VALIDATOR === "true") can avoid surprises.
23-27: Prefer deriving the chain name from the provider instead ofhre.globalOptions.networkBoth
deployCRISPContractsanddeployVerifierusehre.globalOptions.network(and the finalstoreDeploymentArgsforMockRISC0Verifiercalls it directly). Per previous Hardhat v3 experience in this repo,hre.globalOptions.networkcan beundefinedin some contexts, which would lead to deployments being written under an unexpected key or failing downstream.It would be more robust to centralize chain resolution, e.g. via the signer’s provider:
const { ethers } = await hre.network.connect(); const [signer] = await ethers.getSigners(); const chain = (await signer.provider?.getNetwork())?.name ?? "localhost";…and then consistently pass that
chainvariable into allreadDeploymentArgsandstoreDeploymentArgscalls (including theMockRISC0Verifierbranch).Also applies to: 114-115, 145-165
packages/enclave-sdk/src/greco.ts (1)
23-37: NewCircuitInputs/ params structure looks coherent, but bounds ordering deserves a quick sanity checkThe refactor to
CircuitInputsplus{ bounds, crypto }Paramsmirrors the Greco/Noir struct layout and should serialize cleanly into the circuit. One thing that stands out is some “low” vs “up” bounds indefaultParams:
r1_low_bounds: ["261", "258"]vsr1_up_bounds: ["260", "258"]k1_low_bound: "5"vsk1_up_bound: "4"If these are fed into
range_check_2bounds(low, up, ...)-style constraints, havinglow > upwould make the range empty. If the Noir library instead interprets them differently (e.g. the names are historical and order doesn’t matter), this may be fine — but the mismatch with the field names is non‑obvious.I’d recommend double-checking these values against the Greco circuit (
Paramsdefinition and range checks) or tests, and if they are indeed swapped, correcting them indefaultParamsto avoid subtle unsatisfiable constraints.Also applies to: 46-69, 71-93
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/enclave-sdk/tests/fixtures/pubkey.binis excluded by!**/*.binpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.github/workflows/ci.yml(3 hunks)circuits/crates/libs/greco/README.md(1 hunks)circuits/crates/libs/polynomial/README.md(1 hunks)circuits/crates/libs/safe/Nargo.toml(1 hunks)circuits/crates/libs/safe/README.md(1 hunks)examples/CRISP/client/vite.config.ts(1 hunks)examples/CRISP/enclave.config.yaml(2 hunks)examples/CRISP/packages/crisp-contracts/deploy/crisp.ts(3 hunks)examples/CRISP/packages/crisp-contracts/hardhat.config.ts(2 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(2 hunks)examples/CRISP/packages/crisp-sdk/package.json(1 hunks)examples/CRISP/packages/crisp-sdk/src/vote.ts(3 hunks)examples/CRISP/server/.env.example(1 hunks)packages/enclave-sdk/package.json(1 hunks)packages/enclave-sdk/src/enclave-sdk.ts(5 hunks)packages/enclave-sdk/src/greco.ts(4 hunks)packages/enclave-sdk/tests/sdk.test.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: .github/workflows/ci.yml:202-206
Timestamp: 2025-08-27T13:49:03.811Z
Learning: In GitHub Actions workflows for Noir circuits, the user prefers to use "stable" toolchain with noir-lang/noirup action rather than pinning to specific versions like 1.0.0-beta.11, and prefers to update the noirup action version (e.g., to 0.1.4) to get tooling improvements.
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 666
File: examples/CRISP/contracts/CRISPChecker.sol:9-9
Timestamp: 2025-08-29T16:46:50.289Z
Learning: In hashcloak/semaphore-contracts-noir package, the interface is still named ISemaphore, not ISemaphoreNoir. The Noir support was added via Noir verifier contracts while keeping the original ISemaphore interface name.
📚 Learning: 2025-10-10T12:56:40.538Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 830
File: templates/default/README.md:123-128
Timestamp: 2025-10-10T12:56:40.538Z
Learning: In the Enclave repository, the hard-coded Hardhat development private key `0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80` is acceptable in template README files and documentation for local testing/interaction purposes.
Applied to files:
examples/CRISP/server/.env.example
📚 Learning: 2024-10-23T01:59:27.215Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: tests/basic_integration/test.sh:21-21
Timestamp: 2024-10-23T01:59:27.215Z
Learning: In `tests/basic_integration/test.sh`, the hardcoded `CIPHERNODE_SECRET` is acceptable for testing purposes and does not need to be changed.
Applied to files:
examples/CRISP/server/.env.example
📚 Learning: 2025-09-11T13:21:31.031Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/tasks/utils.ts:7-8
Timestamp: 2025-09-11T13:21:31.031Z
Learning: In Hardhat v3, the task API syntax has changed significantly from v2. The new syntax uses:
- `.addOption({ name, description, defaultValue, type })` instead of `.addOptionalParam()`
- `.setAction(async () => ({ default: (args, hre) => { ... } }))` instead of direct `.setAction((args, hre) => { ... })`
- `.build()` is required to finalize task definitions
- `ArgumentType.STRING` is used for option types instead of `types.string`
Applied to files:
examples/CRISP/packages/crisp-contracts/hardhat.config.ts
📚 Learning: 2025-09-11T13:09:03.800Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts:30-31
Timestamp: 2025-09-11T13:09:03.800Z
Learning: In Hardhat v3 deployment scripts, use `(await signer.provider?.getNetwork())?.name ?? "localhost"` instead of `hre.globalOptions.network` to reliably get the network name, as `hre.globalOptions.network` can be undefined in some contexts.
Applied to files:
examples/CRISP/packages/crisp-contracts/hardhat.config.tsexamples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.tsexamples/CRISP/packages/crisp-contracts/deploy/crisp.ts
📚 Learning: 2025-11-05T14:12:57.814Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 963
File: examples/CRISP/client/package.json:25-25
Timestamp: 2025-11-05T14:12:57.814Z
Learning: In the Enclave/CRISP codebase, `enclave-e3/sdk` and `crisp-e3/sdk` are different packages: `enclave-e3/sdk` is the general Enclave SDK, while `crisp-e3/sdk` is the CRISP-specific SDK. The CRISP client (`examples/CRISP/client`) intentionally depends on `enclave-e3/sdk`, not `crisp-e3/sdk`.
Applied to files:
packages/enclave-sdk/package.jsonexamples/CRISP/packages/crisp-sdk/package.jsonexamples/CRISP/packages/crisp-contracts/deploy/crisp.ts
📚 Learning: 2025-08-27T13:49:03.811Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: .github/workflows/ci.yml:202-206
Timestamp: 2025-08-27T13:49:03.811Z
Learning: In GitHub Actions workflows for Noir circuits, the user prefers to use "stable" toolchain with noir-lang/noirup action rather than pinning to specific versions like 1.0.0-beta.11, and prefers to update the noirup action version (e.g., to 0.1.4) to get tooling improvements.
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-11-12T10:08:30.720Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 996
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:144-169
Timestamp: 2025-11-12T10:08:30.720Z
Learning: In the CRISP program (examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol), there are plans to move the merkle tree from the Enclave contract to the program contract itself in the future, which would allow direct calls to validateInput to be stored in the merkle tree.
Applied to files:
examples/CRISP/enclave.config.yamlexamples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.tsexamples/CRISP/packages/crisp-contracts/deploy/crisp.ts
📚 Learning: 2025-09-19T11:16:53.825Z
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 752
File: packages/enclave-contracts/contracts/Enclave.sol:15-17
Timestamp: 2025-09-19T11:16:53.825Z
Learning: The Enclave contract in the gnosisguild/enclave repository has not been deployed yet as of September 2025, so storage layout considerations for upgradeable contracts don't apply to current changes.
Applied to files:
examples/CRISP/enclave.config.yaml
📚 Learning: 2025-11-08T01:31:47.505Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 969
File: packages/enclave-sdk/src/enclave-sdk.ts:101-103
Timestamp: 2025-11-08T01:31:47.505Z
Learning: In packages/enclave-sdk/src/enclave-sdk.ts, the TRBFV protocol parameter setup is intentionally incomplete as of PR #969. The constructor sets BFV_THRESHOLD parameters for TRBFV, but the encryption methods (encryptNumber, encryptVector, encryptNumberAndGenInputs, encryptVectorAndGenInputs) intentionally only support BFV and will throw "Protocol not supported" for TRBFV until future implementation is completed.
Applied to files:
packages/enclave-sdk/src/enclave-sdk.tspackages/enclave-sdk/src/greco.tspackages/enclave-sdk/tests/sdk.test.ts
📚 Learning: 2025-09-22T15:08:29.814Z
Learnt from: ozgurarmanc
Repo: gnosisguild/enclave PR: 734
File: packages/circuits/crates/libs/polynomial/src/lib.nr:140-155
Timestamp: 2025-09-22T15:08:29.814Z
Learning: Greco (packages/circuits/crates/libs/greco/src/lib.nr) performs range_check_1bound/2bounds on all polynomials (u, e0/e1, k1, pk*, r*, p*) before serialization; packer/flatten rely on these bounds, so per-limb asserts inside packer are unnecessary in this crate’s flow.
Applied to files:
packages/enclave-sdk/src/greco.ts
📚 Learning: 2025-08-27T13:49:48.617Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 648
File: examples/CRISP/circuits/src/main.nr:40-44
Timestamp: 2025-08-27T13:49:48.617Z
Learning: In CRISP circuits using Greco library, the binary check `assert(0 == b * (qmt - b))` for polynomial k1 should not be applied to all coefficients - only specific coefficients require this constraint, such as `k1.coefficients[2048 - 1]`.
Applied to files:
packages/enclave-sdk/src/greco.ts
📚 Learning: 2024-10-23T01:59:42.967Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:274-274
Timestamp: 2024-10-23T01:59:42.967Z
Learning: In the `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs` file and other test files within this project, hardcoding `CIPHERNODE_SECRET` is acceptable for testing purposes.
Applied to files:
packages/enclave-sdk/tests/sdk.test.ts
📚 Learning: 2024-10-22T02:10:34.864Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:82-83
Timestamp: 2024-10-22T02:10:34.864Z
Learning: In the file `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs`, when reviewing test functions like `generate_pk_share`, minor performance optimizations (e.g., minimizing mutex locks) are not a priority.
Applied to files:
packages/enclave-sdk/tests/sdk.test.ts
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 133
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:137-139
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs`, using the same RNG instance `rng_test` for generating multiple key shares without advancing its state is acceptable.
Applied to files:
packages/enclave-sdk/tests/sdk.test.ts
📚 Learning: 2024-10-22T02:36:01.448Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/data/Cargo.toml:0-0
Timestamp: 2024-10-22T02:36:01.448Z
Learning: In `packages/ciphernode/data/Cargo.toml`, `actix-rt` is only used in tests and should remain in `[dev-dependencies]`.
Applied to files:
circuits/crates/libs/safe/Nargo.toml
🧬 Code graph analysis (4)
examples/CRISP/packages/crisp-contracts/hardhat.config.ts (2)
packages/enclave-contracts/tasks/utils.ts (1)
cleanDeploymentsTask(11-26)packages/enclave-contracts/tasks/ciphernode.ts (3)
ciphernodeAdd(11-149)ciphernodeAdminAdd(290-500)ciphernodeMintTokens(197-288)
packages/enclave-sdk/src/enclave-sdk.ts (2)
packages/enclave-sdk/src/utils.ts (1)
SDKError(9-17)crates/wasm/src/lib.rs (4)
bfv_encrypt_number(32-42)bfv_encrypt_vector(62-72)bfv_verifiable_encrypt_number(93-108)bfv_verifiable_encrypt_vector(129-144)
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (1)
packages/enclave-contracts/scripts/utils.ts (2)
storeDeploymentArgs(56-88)readDeploymentArgs(96-110)
packages/enclave-sdk/tests/sdk.test.ts (1)
packages/enclave-sdk/src/index.ts (1)
FheProtocol(47-47)
🪛 dotenv-linter (4.0.0)
examples/CRISP/server/.env.example
[warning] 18-18: [UnorderedKey] The E3_PROGRAM_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
⏰ 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_e3_support_dev
- GitHub Check: build_e3_support_risc0
- GitHub Check: test_contracts
- GitHub Check: build_enclave_cli
- GitHub Check: build_sdk
- GitHub Check: integration_prebuild
- GitHub Check: test_net
- GitHub Check: rust_integration
- GitHub Check: crisp_rust_unit
- GitHub Check: rust_unit
🔇 Additional comments (24)
examples/CRISP/enclave.config.yaml (1)
6-6: Contract address updated consistently.The e3_program address is updated to match the new deployment across configuration files.
examples/CRISP/server/.env.example (1)
18-18: Address updated consistently with enclave.config.yaml.The E3_PROGRAM_ADDRESS is aligned across both configuration files.
examples/CRISP/packages/crisp-contracts/hardhat.config.ts (1)
78-83: Tasks array wiring looks consistent with Hardhat v3 task exportsThe imported tasks are simply listed in the
tasksarray; no behavior change and this matches the new task registration style used elsewhere in the repo.examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (1)
42-51: CRISPProgram deployment smoke test is straightforward and usefulUsing fixed non‑zero addresses plus
zeroHashas a minimal constructor check is fine as a lightweight “does it deploy” test without coupling to the full verifier stack.examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (1)
116-143: Idempotent verifier deployments via read/storeDeploymentArgs are a solid improvementThe updated
deployVerifierthat:
- checks for an existing
RiscZeroGroth16Verifier/MockRISC0VerifierviareadDeploymentArgs,- logs and reuses the existing address when present,
- and records fresh deployments with
storeDeploymentArgs,makes repeated runs of the deploy script much safer and faster. The separation of real vs mock verifiers via
useMockVerifieris also clear.Also applies to: 145-152, 153-159, 167-168
examples/CRISP/packages/crisp-sdk/src/vote.ts (1)
279-289: Consistent switch tokeccakZKacross prove/verifyThe move to
{ keccakZK: true }is applied consistently to both proof generation functions andverifyProof, and backend creation/destruction remains symmetric, so this change looks correct from the SDK’s perspective. Please just confirm that the upgradedUltraHonkBackendAPI indeed expects thekeccakZKflag name and defaults (and that circuits are compiled with the corresponding hash configuration), so proofs remain compatible with on-chain/off-chain verifiers.Also applies to: 291-303, 313-321
packages/enclave-sdk/src/greco.ts (1)
100-115: Polynomial conversion helpers are straightforward and type-safe
convertToPolynomialandconvertToPolynomialArraysimply wrap string arrays into thePolynomialshape and are used consistently for allCircuitInputsslices. This keeps the Noir callsite clean and matches thePolynomialtype definition.packages/enclave-sdk/package.json (1)
40-41: LGTM! Dependency versions consistent with CRISP SDK.The Noir dependency versions match those in
examples/CRISP/packages/crisp-sdk/package.json, ensuring consistency across the codebase..github/workflows/ci.yml (2)
269-269: Nargo version pinned to beta.15 across all workflow steps.The Nargo toolchain has been consistently updated from v1.0.0-beta.3 to v1.0.0-beta.15 across all workflow steps (integration_prebuild, crisp_e2e, and test_enclave_circuits).
Note: Based on learnings, there's a preference for using "stable" toolchain rather than pinning to specific beta versions. However, pinning to a specific beta version provides better reproducibility for CI, which may be the intended trade-off here.
452-452: LGTM! Nargo version consistent across all CI steps.The toolchain version is consistently set to v1.0.0-beta.15 in all workflow steps.
Also applies to: 524-524
circuits/crates/libs/polynomial/README.md (1)
25-32: LGTM! Toolchain version documentation consistent.The version references align with those updated in other library READMEs throughout the repository.
circuits/crates/libs/greco/README.md (1)
89-96: LGTM! Toolchain documentation fully updated.All version references have been consistently updated across the repository's library documentation.
circuits/crates/libs/safe/Nargo.toml (1)
10-10: Confirm sha256 v0.2.0 update includes breaking changes—verify codebase compatibility.The sha256 v0.2.0 tag exists and the release notes document a breaking change: "switch to new bit-shift semantic". Review your code to ensure compatibility with this semantic change, particularly if your implementation relies on bit-shift operations.
examples/CRISP/packages/crisp-sdk/package.json (1)
47-49: The review comment's concern about version bumps is invalid for pre-release software.The verification confirms these are the exact versions officially recommended together in Noir's documentation. Both
@noir-lang/noir_js@1.0.0-beta.15and@aztec/bb.js@3.0.0-nightly.20251104are already actively used in the codebase (enclave-sdk, CRISP SDK) for zero-knowledge proof generation and circuit compilation.Pre-release software like Noir and Aztec use rapid iteration versioning during beta/nightly phases; calling a beta.3→beta.15 jump a "major version bump" mischaracterizes how this versioning works. Aztec explicitly documents that breaking changes occur frequently during active development—this is expected and managed through the pinned versions. Exact pinning (rather than caret ranges) is the correct practice for pre-release software.
examples/CRISP/client/vite.config.ts (1)
25-25: Build verification inconclusive—requires full development environment with Rust toolchain.The @aztec/bb.js removal from the exclude list is confirmed (commit c302bab). However, the build fails due to missing prerequisite builds in the sandbox environment (Rust toolchain unavailable for WASM compilation of @crisp-e3/sdk). This prevents a definitive test of whether pre-bundling @aztec/bb.js works correctly.
Please verify the CRISP client builds successfully in your full development environment:
cd examples/CRISP/client pnpm install pnpm buildEnsure all workspace dependencies (including Rust-based WASM builds) are available and the build completes without errors.
circuits/crates/libs/safe/README.md (1)
64-71: I need to search more specifically for Noir's public releases to verify whether 1.0.0-beta.15 actually exists, as the initial result was inconclusive.Versions verified as accurate and available.
Noir 1.0.0-beta.15 is the current documented version, and the official Noir tutorial explicitly references @aztec/bb.js@3.0.0-nightly.20251104. The documentation update is correct. Both versions are intentionally newer and available as expected.
packages/enclave-sdk/tests/sdk.test.ts (3)
42-56: LGTM - Proof generation tests updated consistently.The proof generation tests have been properly updated to use the TRBFV protocol while maintaining the same test structure and assertions.
Also applies to: 70-84
18-31: TRBFV implementation is complete and functional per current code.The complete encryption chain now fully supports TRBFV. The
getProtocolParams()method handles both BFV and TRBFV protocols, returning protocol-specific parameters (INSECURE_SET_512_10_1 for TRBFV). All encryption methods (encryptNumber,encryptVector,encryptNumberAndGenInputs,encryptVectorAndGenInputs) pass these parameters to WASM functions without protocol validation, allowing TRBFV to execute successfully.The learning note from PR #969 is outdated—TRBFV support is no longer incomplete. However, verify that the test's output size assertion (9,242 bytes at lines 39 and 67) matches actual TRBFV encryption output, as different parameter sets may produce different ciphertext sizes.
33-41: Verify the expected encryption output length for TRBFV parameters.The expected encrypted output length changed from 27,674 to 9,242 bytes (66% reduction), which aligns with the smaller degree parameter in TRBFV (512 vs 2048 for BFV). However, the hardcoded value should be verified.
Run the following script to verify the actual output length:
Also applies to: 58-68
packages/enclave-sdk/src/enclave-sdk.ts (5)
95-98: LGTM - Protocol validation improves robustness.The addition of protocol validation in the constructor prevents invalid protocol configurations at initialization time. The error message clearly indicates which protocol value was invalid.
158-172: LGTM - Encryption logic correctly simplified.The removal of the switch statement is valid because
getProtocolParams()handles protocol-specific parameter selection. Both BFV and TRBFV use the same underlyingbfv_encrypt_numberfunction with different parameter sets, which is the correct design for Threshold BFV.Based on learnings
180-194: LGTM - Vector encryption simplified consistently.The vector encryption method follows the same pattern as
encryptNumber, correctly delegating protocol-specific behavior to the parameterized BFV function.
203-223: LGTM - Verifiable encryption correctly simplified.The method properly destructures the WASM function output and parses the circuit inputs from JSON. The simplification maintains correct functionality while improving code clarity.
253-273: LGTM - Vector verifiable encryption simplified consistently.The changes mirror
encryptNumberAndGenInputs, maintaining consistency across all encryption methods in the SDK. All four encryption methods now follow the same clean pattern of calling parameterized BFV functions.
yea this makes sense. Perhaps simply encryption proving with greco could be more of a guideline for users rather than something they'd use directly |
|
4 participants
Closes #1013
Skipping the line limit as the new verifier is quite heavy.
Summary by CodeRabbit
New Features
Bug Fixes
Chores