feat: store merkle tree on program [skip-line-limit]#1027
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughThis PR moves input Merkle-tree management from the Enclave into program-level (per-e3) logic, replaces LeanIMT with LazyIMT/zk-kit-imt variants, updates validateInput to accept an e3Id, removes enclave-level input accessors and InputPublished wiring, and updates callers, indexer, SDK, templates, and deployment scripts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Enclave
participant Program as E3Program
rect rgb(200,220,240)
Note over User,Enclave: Old flow — Enclave-managed inputs
User->>Enclave: publishInput(e3Id, data)
Enclave->>Enclave: insert into inputs IMT, increment count
Enclave->>User: emit InputPublished(...)
end
rect rgb(220,240,200)
Note over User,Program: New flow — Program-managed inputs
User->>Enclave: publishInput(e3Id, data)
Enclave->>Program: validateInput(e3Id, sender, data)
Program->>Program: votes[e3Id].insert/update leaf
Program->>Program: emit InputPublished(e3Id, data, index)
Program-->>Enclave: validated input
end
sequenceDiagram
participant VerifierFlow
rect rgb(240,200,200)
Note over VerifierFlow: Old verification root source
VerifierFlow->>VerifierFlow: inputRoot = enclave.getInputRoot(e3Id)
end
rect rgb(200,240,200)
Note over VerifierFlow: New verification root source
VerifierFlow->>VerifierFlow: inputRoot = votes[e3Id]._root(TREE_DEPTH)
VerifierFlow->>VerifierFlow: verify using program-level root
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
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)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-11-12T10:08:30.720ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol (3)
150-203: Fix first‑vote detection, add missingoverridekeyword, and update stale commentThree issues confirmed that require immediate fixes:
Inverted first‑vote check (lines 174–176)
The condition
isFirstVote = oldCiphertext != 0is backwards. When a slot is first used,voteSlots[e3Id][slot]defaults to0, sovotes[e3Id].elements[0]is also0. This means first votes incorrectly evaluateisFirstVoteasfalse, causing the first vote to call_update(voteHash, 0)instead of_insert. Subsequent votes then incorrectly call_insert. Flip the condition toisFirstVote = oldCiphertext == 0so first votes insert and subsequent votes update:- bool isFirstVote = oldCiphertext != 0; + bool isFirstVote = oldCiphertext == 0;Missing
overridekeyword onvalidateInput(line 151)The
verifyfunction already usesoverride(line 205), butvalidateInputdoes not, despite implementing theIE3Programinterface. Addoverride:function validateInput(uint256 e3Id, address, bytes memory data) + override external returns (bytes memory input)Stale comment (line 193)
The input Merkle tree now lives in
CRISPProgram, not in Enclave. Update the comment:- // return the vote so that it can be stored in Enclave's input merkle tree + // return the vote for IE3Program compatibility; the input Merkle tree is maintained here
13-53: Fix inverted first-vote detection logic and add missingoverridekeywordThe first-vote check at line 177 is backwards:
bool isFirstVote = oldCiphertext != 0;Should be
== 0(when no prior ciphertext exists, it's the first vote). Currently the code attempts_insert()when there's a prior vote and_update()on empty slots—the opposite of intended behavior.Additionally,
validateInputat line 151 is missing theoverridekeyword despite bearing@inheritdoc IE3Program, whereasverify(line 246) correctly declares it. This inconsistency violates strict interface compliance.Minor: the comment at line 195 ("stored in Enclave's input merkle tree") is stale; votes are now stored in the program-level tree.
252-259: Update MockCRISPProgram.sol to use program-levelinputRootfor consistencyThe verification found that while CRISPProgram.sol correctly uses the new program-level input root (
votes[e3Id]._root()), the fallback MockCRISPProgram.sol still depends on the old Enclave-level approach (enclave.getInputRoot(e3Id)). Since MockCRISPProgram is deployed conditionally in the deployment flow, this inconsistency should be resolved.Location: examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol, line 201
Change:
bytes32 inputRoot = bytes32(enclave.getInputRoot(e3Id));to:
bytes32 inputRoot = bytes32(votes[e3Id]._root());Note: The template (templates/default/contracts/MyProgram.sol) intentionally uses the old approach as a reference implementation for new programs; no changes needed there.
🧹 Nitpick comments (6)
packages/enclave-contracts/scripts/deployAndSave/enclave.ts (1)
139-147: Remove obsoleteposeidonT3Addressfrom the upgrade helper API
upgradeAndSaveEnclavestill takesposeidonT3Addressin the destructured args and type, but it is no longer used anywhere in the function. This leaves a misleading, dead parameter in a public helper whose purpose is now independent of Poseidon.I recommend deleting it from the signature and type to keep the API consistent with the rest of the PR and avoid confusion for callers.
-export const upgradeAndSaveEnclave = async ({ - poseidonT3Address, - ownerAddress, - hre, -}: { - poseidonT3Address: string; - ownerAddress: string; - hre: HardhatRuntimeEnvironment; -}): Promise<{ enclave: Enclave; implementationAddress: string }> => { +export const upgradeAndSaveEnclave = async ({ + ownerAddress, + hre, +}: { + ownerAddress: string; + hre: HardhatRuntimeEnvironment; +}): Promise<{ enclave: Enclave; implementationAddress: string }> => {After this, adjust any call sites to stop passing
poseidonT3Address.packages/enclave-contracts/contracts/Enclave.sol (1)
323-341: Program-levelvalidateInputusage aligns with Merkle-tree extraction; consider interface/gas tweaks.Routing
publishInputthroughe3.e3Program.validateInput(e3Id, msg.sender, data)is a good fit for moving input policy and Merkle tree management to the program layer: the program now has bothe3Idand caller address, which matches the design goal (e.g. CRISP’s “latest per-user vote” policy). Based on learningsTwo small follow-ups to consider:
- The interface currently returns
bytes memory inputbut the value is ignored here. If the canonicalized input is only ever used within the program (to update its own tree/state), you might simplify the interface to not return anything, or document that the return value is intentionally ignored at the Enclave level.- Because IE3Program takes
bytes memory data, this call incurs a calldata→memory copy. If you expect large inputs or heavy traffic, you may want to evaluate switching the IE3Program signature tobytes calldataand updating this call accordingly to avoid that copy.examples/CRISP/server/src/server/models.rs (1)
167-169: Consider removing the commented-out ciphertext_inputs field from E3Commenting out
ciphertext_inputsonE3while introducing it onE3Crispreflects the new design, but leaving the old field commented can cause confusion. Once the migration is stable, consider deleting this commented block to keep the model surface clear.examples/CRISP/server/src/server/repo.rs (1)
94-109: Remove legacy insert_ciphertext_input implementation comment blockThe old
insert_ciphertext_inputimplementation against the enclaveE3is now fully superseded by the E3Crisp-based version. Keeping it as a large commented block adds noise; consider deleting it to avoid confusion about which path is actually in use.examples/CRISP/server/src/server/indexer.rs (1)
399-423: register_input_published correctly wires CRISP InputPublished into the repositoryThis listener:
- Listens on the CRISP contract via
EventListener.- Converts
e3Id/indextou64.- Appends
(vote, index)toE3Crisp.ciphertext_inputsthroughCrispE3Repository.That matches the storage changes in
repo.rsand the event definition inmodels.rs. One small improvement would be to switch theprintln!to the structuredlogmacros (e.g.,info!) to keep logging uniform with the rest of the server.packages/enclave-contracts/contracts/test/MockE3Program.sol (1)
36-46: Name the newvalidateInputparameter for clarityThe added leading
uint256parameter correctly matches the updated IE3Program signature, but leaving it unnamed makes the call-site intent less obvious (it’s presumablye3Id).You could slightly improve readability (and avoid “unused parameter” confusion in some tooling) by doing e.g.:
- function validateInput( - uint256, + function validateInput( + uint256 /* e3Id */, address sender, bytes memory data ) external pure returns (bytes memory input) {or by explicitly naming it
uint256 e3Idif you later decide to use it in tests.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
crates/evm-helpers/src/events.rs(0 hunks)crates/indexer/src/indexer.rs(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol(7 hunks)examples/CRISP/packages/crisp-contracts/deployed_contracts.json(1 hunks)examples/CRISP/packages/crisp-contracts/package.json(1 hunks)examples/CRISP/packages/crisp-contracts/remappings.txt(1 hunks)examples/CRISP/server/src/server/indexer.rs(6 hunks)examples/CRISP/server/src/server/mod.rs(1 hunks)examples/CRISP/server/src/server/models.rs(4 hunks)examples/CRISP/server/src/server/repo.rs(4 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json(1 hunks)packages/enclave-contracts/contracts/Enclave.sol(1 hunks)packages/enclave-contracts/contracts/interfaces/IE3Program.sol(1 hunks)packages/enclave-contracts/contracts/interfaces/IEnclave.sol(0 hunks)packages/enclave-contracts/contracts/test/MockE3Program.sol(1 hunks)packages/enclave-contracts/package.json(0 hunks)packages/enclave-contracts/scripts/deployAndSave/enclave.ts(2 hunks)packages/enclave-contracts/scripts/deployEnclave.ts(0 hunks)packages/enclave-contracts/tasks/enclave.ts(0 hunks)packages/enclave-contracts/test/Enclave.spec.ts(0 hunks)
💤 Files with no reviewable changes (6)
- packages/enclave-contracts/test/Enclave.spec.ts
- crates/evm-helpers/src/events.rs
- packages/enclave-contracts/package.json
- packages/enclave-contracts/contracts/interfaces/IEnclave.sol
- packages/enclave-contracts/scripts/deployEnclave.ts
- packages/enclave-contracts/tasks/enclave.ts
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 996
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:144-169
Timestamp: 2025-11-12T10:08:30.720Z
Learning: In the CRISP program (examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol), there are plans to move the merkle tree from the Enclave contract to the program contract itself in the future, which would allow direct calls to validateInput to be stored in the merkle tree.
📚 Learning: 2024-10-29T01:03:50.414Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/config/src/app_config.rs:21-26
Timestamp: 2024-10-29T01:03:50.414Z
Learning: In `packages/ciphernode/config/src/app_config.rs`, the `rpc_url` field in the `ChainConfig` struct is not considered sensitive and does not need to be encrypted.
Applied to files:
examples/CRISP/server/src/server/mod.rs
📚 Learning: 2025-11-12T10:08:30.720Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 996
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:144-169
Timestamp: 2025-11-12T10:08:30.720Z
Learning: In the CRISP program (examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol), there are plans to move the merkle tree from the Enclave contract to the program contract itself in the future, which would allow direct calls to validateInput to be stored in the merkle tree.
Applied to files:
examples/CRISP/packages/crisp-contracts/deployed_contracts.jsonexamples/CRISP/server/src/server/indexer.rsexamples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.solpackages/enclave-contracts/contracts/interfaces/IE3Program.solpackages/enclave-contracts/contracts/Enclave.sol
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Applied to files:
crates/indexer/src/indexer.rsexamples/CRISP/server/src/server/indexer.rsexamples/CRISP/server/src/server/models.rs
📚 Learning: 2025-04-30T06:25:14.721Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 372
File: packages/ciphernode/events/src/eventbus.rs:15-15
Timestamp: 2025-04-30T06:25:14.721Z
Learning: EnclaveEvent implements Display in packages/ciphernode/events/src/enclave_event/mod.rs, which satisfies the Event trait requirement. Static analysis tools may incorrectly flag implementations as missing when they do exist.
Applied to files:
crates/indexer/src/indexer.rs
📚 Learning: 2024-10-22T03:39:29.448Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/evm/src/enclave_sol_reader.rs:87-89
Timestamp: 2024-10-22T03:39:29.448Z
Learning: In the `ciphernode` project, specifically in `packages/ciphernode/evm/src/enclave_sol_reader.rs`, the method `EnclaveSolReader::attach` should be retained even if it directly calls `EvmEventReader::attach` without additional processing. Avoid suggesting its removal in future reviews.
Applied to files:
crates/indexer/src/indexer.rsexamples/CRISP/server/src/server/models.rs
📚 Learning: 2024-10-22T03:30:45.478Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/data/src/repository.rs:58-60
Timestamp: 2024-10-22T03:30:45.478Z
Learning: In `packages/ciphernode/data/src/repository.rs`, the `write` method in `Repository<T>` must be synchronous and should not return errors.
Applied to files:
examples/CRISP/server/src/server/repo.rs
📚 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:
packages/enclave-contracts/scripts/deployAndSave/enclave.tspackages/enclave-contracts/contracts/Enclave.sol
📚 Learning: 2025-09-11T13:02:56.353Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts:6-6
Timestamp: 2025-09-11T13:02:56.353Z
Learning: In Hardhat v3, the import path "hardhat/types/hre" may be valid for importing HardhatRuntimeEnvironment, unlike in Hardhat v2 where the standard path was "hardhat/types".
Applied to files:
packages/enclave-contracts/scripts/deployAndSave/enclave.ts
📚 Learning: 2025-09-11T13:02:56.353Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts:6-6
Timestamp: 2025-09-11T13:02:56.353Z
Learning: In Hardhat v3, the import path "hardhat/types/hre" is valid for importing HardhatRuntimeEnvironment, which is different from Hardhat v2 where "hardhat/types" was the standard path.
Applied to files:
packages/enclave-contracts/scripts/deployAndSave/enclave.ts
📚 Learning: 2025-09-11T13:02:56.353Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts:6-6
Timestamp: 2025-09-11T13:02:56.353Z
Learning: In Hardhat v3, the correct import path for HardhatRuntimeEnvironment is "hardhat/types/hre", not "hardhat/types" as it was in Hardhat v2. The import structure has changed between major versions.
Applied to files:
packages/enclave-contracts/scripts/deployAndSave/enclave.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, hre.network.name is not available anymore. Use hre.globalOptions.network to get the network name instead.
Applied to files:
packages/enclave-contracts/scripts/deployAndSave/enclave.ts
🧬 Code graph analysis (2)
examples/CRISP/server/src/server/indexer.rs (4)
examples/CRISP/server/src/server/program_server_request.rs (1)
run_compute(47-71)examples/CRISP/server/src/server/repo.rs (4)
store(35-37)store(78-80)new(20-22)new(63-65)crates/indexer/src/indexer.rs (4)
store(356-359)new(44-48)new(113-115)new(175-191)crates/evm-helpers/src/listener.rs (2)
contract_address(99-99)create_contract_listener(97-104)
examples/CRISP/server/src/server/models.rs (1)
examples/CRISP/server/src/server/indexer.rs (8)
event(48-48)event(98-98)event(187-187)event(190-190)event(277-277)event(295-295)event(406-406)event(416-416)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: rust_integration
- GitHub Check: rust_unit
- GitHub Check: build_enclave_cli
- GitHub Check: test_net
- GitHub Check: Build & Push Image
🔇 Additional comments (18)
examples/CRISP/packages/crisp-contracts/deployed_contracts.json (1)
110-110: Confirm the MockRISC0Verifier address update is intentional and documented.The address for MockRISC0Verifier has been updated, likely due to contract redeployment after changes to dependencies or initialization. Ensure this reflects an intentional redeploy in the test environment and verify that:
- The new address (0xa85233C63b9Ee964Add6F2cffe00Fd84eb32338f) is correct and has been validated.
- Any systems or scripts that depend on the previous address have been updated accordingly.
- The AI summary indicates HonkVerifier, CRISPProgram, and CRISPInputValidator entries were also removed from this file; confirm these removals are intentional and coordinated with the merkle tree extraction changes.
packages/enclave-contracts/scripts/deployAndSave/enclave.ts (2)
77-82: Factory switch away from Poseidon linking looks goodUsing
ethers.getContractFactory("Enclave", signer)for deployment (without library linking) aligns with removing Poseidon from the Enclave level and keeps the rest of the flow unchanged. No issues from this change indeployAndSaveEnclave.
165-171: Upgrade deployment path matches the new deployment patternThe upgrade flow now mirrors the deployment path by using
ethers.getContractFactory("Enclave", signer)and deploying a fresh implementation without Poseidon linking, then attaching viaEnclaveFactory.connectto the proxy. This is consistent and keeps the on-chain records updated correctly.crates/indexer/src/indexer.rs (1)
15-19: InputPublished removal from core indexer import is consistentDropping
InputPublishedfrom theeventsimport cleanly reflects that the core enclave indexer no longer processes input events; only activation and output events remain wired, which aligns with pushing input handling up to the program/server layer.examples/CRISP/server/src/server/mod.rs (1)
42-49: start_indexer call updated correctly for crisp_addressPassing
&CONFIG.e3_program_addressas the new fourth parameter matches the updatedstart_indexersignature and ensures the CRISP listener is pointed at the program contract while reusing the same shared store.examples/CRISP/server/src/server/models.rs (3)
7-7: alloy::sol import is appropriate for sol! usageThe
use alloy::sol;import is the correct way to enable thesol!macro used later forInputPublished.
178-189: E3Crisp.ciphertext_inputs aligns with program-level input trackingAdding
ciphertext_inputs: Vec<(Vec<u8>, u64)>toE3Crispgives the CRISP-specific repository a place to store per-round inputs, decoupled from the enclave’s generic E3 model. This matches the goal of making input policies program-specific.Based on learnings
205-207: InputPublished event definition matches server indexer expectationsThe
sol!-definedInputPublished(uint256 indexed e3Id, bytes vote, uint256 index)event provides the exact shape used in the CRISP indexer (e3Id/vote/indexwith.to::<u64>()conversions), so event decoding and repository writes should interoperate cleanly with the Solidity CRISPProgram.Based on learnings
examples/CRISP/server/src/server/repo.rs (3)
111-124: New CRISP-level insert_ciphertext_input correctly targets E3CrispThe updated
insert_ciphertext_inputwriting intocrisp_keyand appending(vote, index)toE3Crisp.ciphertext_inputsis consistent with the new program-level input storage design and with how the CRISP indexer calls this viaregister_input_published.
131-142: Initializing ciphertext_inputs to an empty Vec is necessaryAdding
ciphertext_inputs: vec![]ininitialize_roundensures later inserts viainsert_ciphertext_inputand reads viaget_vote_count/get_ciphertext_inputsnever hit an uninitialized vector, which is the right default.
235-238: get_ciphertext_inputs accessor matches compute path needsExposing
get_ciphertext_inputsas a simple read ofE3Crisp.ciphertext_inputsis exactly what theregister_e3_activatedflow needs to pass inputs intorun_compute, and it keeps the program-server boundary clean.examples/CRISP/server/src/server/indexer.rs (4)
7-14: New InputPublished import from models is appropriateImporting
InputPublishedalongsideCurrentRoundandCustomParamsis required for the new CRISP listener and keeps all server-side event types centralized inmodels.
18-31: SharedStore import enables shared DB across indexers and listenersBringing
SharedStoreinto scope here matches its use inregister_input_publishedandstart_indexer, letting enclave and CRISP listeners operate over the same persisted store.
217-230: E3Activated now pulls inputs from CRISP repository for computeSwitching from enclave-level ciphertext inputs to
repo.get_ciphertext_inputs()before callingrun_computecorrectly uses the CRISP-specific input list that is populated from the program’sInputPublishedevents, aligning compute with the new program-level input policy.Based on learnings
425-461: start_indexer signature and wiring correctly add CRISP listener and shared storeThe revised
start_indexer:
- Accepts
crisp_address: &strand aSharedStore<impl DataStore>, matching the new call inserver::start.- Uses the shared store when constructing
EnclaveIndexer, so all enclave-driven handlers work over the same DB.- Creates a dedicated CRISP contract listener at
crisp_addressand passes the samestoreintoregister_input_published, ensuring InputPublished events update the same CRISP repository seen by the enclave event handlers and HTTP layer.This wiring cleanly completes the move of input handling to the program-level CRISP components.
Based on learnings
packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json (1)
927-927: UpdatedbuildInfoIdreflects recompiled IEnclave artifactThe new
buildInfoIdis expected after changing the interface and recompiling; the ABI contents remain consistent with the current interface in this diff.examples/CRISP/packages/crisp-contracts/remappings.txt (1)
5-5: Lazy IMT remapping aligns with the new dependencyThe alias and path for
@zk-kit/lazy-imt.solin remappings.txt are consistent with the updated package.json dependency and the Solidity import in CRISPProgram.sol. Migration from lean-imt.sol to lazy-imt.sol is complete within this package—no stale imports remain in crisp-contracts.examples/CRISP/packages/crisp-contracts/package.json (1)
44-46: ****The original review comment suggested confirming the version choice and checking for a non-beta alternative. However, 2.0.0-beta.12 is the latest published version of @zk-kit/lazy-imt.sol, and there is no non-beta (stable) release available. The pinned version in package.json is appropriate as written—it targets the current latest release. No action is needed.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
templates/default/contracts/MyProgram.sol (1)
80-94: Add access control and E3 existence checks tovalidateInput, and update NatSpecRight now any caller can invoke
validateInputfor anye3Idand append leaves to that instance’s IMT, regardless of whether the E3 has been initialized. That’s inconsistent withvalidate, which enforcesauthorizedContracts[msg.sender] || msg.sender == owner(), and it allows arbitrary pollution of the input tree andInputPublishedstream.I recommend:
- Enforce the same caller authorization as
validate.- Ensure the
e3Idhas been initialized (paramsHashes[e3Id] != 0), matchingverify.- Update the NatSpec to document the new
e3Idparameter.For example:
- /// @notice Validates input - /// @param sender The account that is submitting the input. - /// @param data The input to be verified. - /// @return input The input data. + /// @notice Validates input for a given E3 instance and records it in the IMT. + /// @param e3Id The E3 program ID. + /// @param sender The account that is submitting the input. + /// @param data The input to be verified. + /// @return input The input data. function validateInput( uint256 e3Id, address sender, bytes memory data ) external returns (bytes memory input) { + if ( + !authorizedContracts[msg.sender] && + msg.sender != owner() + ) revert CallerNotAuthorized(); + + if (paramsHashes[e3Id] == bytes32(0)) revert E3DoesNotExist(); + if (data.length == 0) revert EmptyInputData(); @@ - input = data; - - inputs[e3Id]._insert(uint256(keccak256(data))); - - emit InputPublished(e3Id, data, inputs[e3Id].numberOfLeaves - 1); + input = data; + + inputs[e3Id]._insert(uint256(keccak256(data))); + + emit InputPublished(e3Id, data, inputs[e3Id].numberOfLeaves - 1);(Optionally you might also want to bind the leaf to
senderby hashing(e3Id, sender, data)instead of justdata.)
🧹 Nitpick comments (2)
templates/default/contracts/MyProgram.sol (2)
12-27: IMT integration pattern looks good; consider tighteninginputsvisibilityThe move to a per‑
e3IdLazyIMTDatamapping withInternalLazyIMTis consistent with the program‑level Merkle tree pattern used in CRISPProgram and aligns with the goal of removing enclave‑level input trees. However, exposinginputsaspublicalso exposes the fullLazyIMTDatastruct, which couples your external ABI to the library’s internal layout.To keep the template more robust, I’d suggest:
- Make the mapping non‑public (
internal/private), and- Add explicit view helpers such as
getInputRoot(uint256 e3Id)and (optionally)getInputLeafCount(uint256 e3Id)that return only what consumers should rely on.This avoids ABI breakage if
LazyIMTDatachanges while still supporting indexers and UIs that need roots/counts.Based on learnings
37-37: Re‑evaluate including fulldatainInputPublishedfor gas and privacyEmitting the entire
bytes datainInputPublishedis convenient for indexers but can become expensive for large inputs and also makes all input payloads permanently visible on‑chain. For a generic template, some consumers might prefer:
- Emitting only a hash (e.g.
bytes32 dataHash) or- Emitting both a compact hash and, optionally, trimmed/structured fields, leaving full data off‑chain.
Consider whether this default should instead emit
keccak256(data)and let concrete programs (like CRISP) override with richer events when they accept the trade‑offs.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
templates/default/contracts/MyProgram.sol(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 996
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:144-169
Timestamp: 2025-11-12T10:08:30.720Z
Learning: In the CRISP program (examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol), there are plans to move the merkle tree from the Enclave contract to the program contract itself in the future, which would allow direct calls to validateInput to be stored in the merkle tree.
📚 Learning: 2025-11-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:
templates/default/contracts/MyProgram.sol
🔇 Additional comments (1)
templates/default/contracts/MyProgram.sol (1)
106-106: Program‑managed IMT root inverifyis directionally correct; confirm guest/journal alignmentSwitching
inputRoottobytes32(inputs[e3Id]._root())correctly reflects the new design where the program owns the input Merkle tree instead of the enclave. Two things to double‑check:
- That the RISC Zero guest program computes the same root using the same
LazyIMTparameters (hashing scheme, insertion order, default empty tree root). Any mismatch will cause otherwise valid proofs to fail.- That the type reinterpretation
bytes32(uint256Root)matches how the guest encodes the root into its journal (no extra hashing or encoding differences).If both sides are wired to the same IMT semantics, this change should be safe and keeps
verifyconsistent with the program‑level tree.Based on learnings
|
❌ License Header Check Failed Some files are missing the required SPDX license header. Please add the following header to the beginning of all You can run Or run |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol (1)
150-157: Critical: validateInput doesn't populate the program-level Merkle tree.This function decodes and returns the vote but fails to:
- Insert the vote into
votes[e3Id]using LazyIMT methods (e.g.,votes[e3Id]._insert(keccak256(vote)))- Track the index in
voteSlots[e3Id][voter]for application-specific policies- Emit the
InputPublishedeventWithout these operations, the
verifyfunction (line 208) will read an empty tree root, breaking the entire computation verification flow and defeating the PR's core objective of moving Merkle tree storage to the program level.Consider this implementation:
-function validateInput(uint256 e3Id, address, bytes memory data) external returns (bytes memory input) { +function validateInput(uint256 e3Id, address voter, bytes memory data) external returns (bytes memory input) { if (data.length == 0) revert EmptyInputData(); (,, bytes memory vote,) = abi.decode(data, (bytes, bytes32[], bytes, address)); + // Insert vote into the Merkle tree + bytes32 voteHash = keccak256(vote); + uint40 index = uint40(votes[e3Id]._insert(voteHash)); + + // Track the vote slot for this voter (enables latest-vote-only policies) + voteSlots[e3Id][voter] = index; + + // Emit event for indexer consumption + emit InputPublished(e3Id, vote, index); + input = vote; }Based on learnings.
🧹 Nitpick comments (3)
templates/default/deploy/default.ts (2)
9-9: Confirm TypeChain output path forMyProgramFactoryimport
../types/factories/contractsassumes generated types live undertypes/factories/contractsand expose an index that re‑exportsMyProgram__factory. For a template, this is sensitive to TypeChain’soutDirsetting (oftentypechain-typesby default). Please double‑check this matches the actual generated layout for new projects so consumers don’t have to tweak the import manually.
45-52: ValidatelinkBytecodeusage and hard‑coded library name for Poseidon linkingUsing
ethers.getContractFactory(MyProgramFactory.abi, MyProgramFactory.linkBytecode(...), owner)is a reasonable way to inject the PoseidonT3 address into the bytecode, but there are two brittle points worth double‑checking:
MyProgramFactory.linkBytecode(...): confirm the generatedMyProgram__factoryfor your TypeChain target actually exposes this helper and that its signature matches how you’re calling it.- Library key
"npm/poseidon-solidity@0.0.5/PoseidonT3.sol:PoseidonT3": this must exactly match the fully‑qualified library name in the compiled artifact’slinkReferences. Any change to the import path or version will silently break linking at compile/deploy time.It may be worth centralizing that fq library name (e.g., exporting it from a small constants module) or deriving it from the artifact to avoid future drift.
templates/default/server/index.ts (1)
238-243: Consider thefromBlockparameter for event listening.The event listener starts from block
0n, which will process all historicalInputPublishedevents since genesis. If the server restarts frequently or the chain has significant history, this could cause:
- Duplicate input processing if sessions aren't persisted
- Slow startup times scanning historical blocks
Consider whether you should:
- Start from the current block if session state is ephemeral
- Track the last processed block if you need to recover from restarts
Example for starting from latest:
const latestBlock = await sdk.getPublicClient().getBlockNumber(); await listenToInputPublishedEvents( sdk.getPublicClient(), PROGRAM_ADDRESS as `0x${string}`, latestBlock );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol(7 hunks)examples/CRISP/server/src/server/repo.rs(5 hunks)packages/enclave-react/src/index.ts(0 hunks)packages/enclave-sdk/src/enclave-sdk.ts(4 hunks)packages/enclave-sdk/src/index.ts(0 hunks)packages/enclave-sdk/src/types.ts(0 hunks)templates/default/deploy/default.ts(3 hunks)templates/default/deployed_contracts.json(2 hunks)templates/default/enclave.config.yaml(1 hunks)templates/default/package.json(1 hunks)templates/default/server/index.ts(3 hunks)templates/default/server/types.ts(1 hunks)templates/default/server/utils.ts(1 hunks)templates/default/tests/integration.spec.ts(1 hunks)
💤 Files with no reviewable changes (3)
- packages/enclave-sdk/src/index.ts
- packages/enclave-react/src/index.ts
- packages/enclave-sdk/src/types.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 996
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:144-169
Timestamp: 2025-11-12T10:08:30.720Z
Learning: In the CRISP program (examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol), there are plans to move the merkle tree from the Enclave contract to the program contract itself in the future, which would allow direct calls to validateInput to be stored in the merkle tree.
📚 Learning: 2025-11-05T14:12:57.814Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 963
File: examples/CRISP/client/package.json:25-25
Timestamp: 2025-11-05T14:12:57.814Z
Learning: In the Enclave/CRISP codebase, `enclave-e3/sdk` and `crisp-e3/sdk` are different packages: `enclave-e3/sdk` is the general Enclave SDK, while `crisp-e3/sdk` is the CRISP-specific SDK. The CRISP client (`examples/CRISP/client`) intentionally depends on `enclave-e3/sdk`, not `crisp-e3/sdk`.
Applied to files:
packages/enclave-sdk/src/enclave-sdk.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:
templates/default/enclave.config.yamltemplates/default/deployed_contracts.json
📚 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:
templates/default/enclave.config.yaml
📚 Learning: 2025-11-12T10:08:30.720Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 996
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:144-169
Timestamp: 2025-11-12T10:08:30.720Z
Learning: In the CRISP program (examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol), there are plans to move the merkle tree from the Enclave contract to the program contract itself in the future, which would allow direct calls to validateInput to be stored in the merkle tree.
Applied to files:
examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol
🧬 Code graph analysis (3)
templates/default/server/types.ts (1)
packages/enclave-sdk/src/types.ts (1)
AllEventTypes(123-123)
templates/default/deploy/default.ts (1)
packages/enclave-contracts/scripts/utils.ts (1)
readDeploymentArgs(96-110)
templates/default/server/index.ts (2)
templates/default/server/types.ts (1)
InputPublishedEvent(9-13)templates/default/server/utils.ts (1)
getCheckedEnvVars(15-28)
🪛 GitHub Actions: License Header Check
templates/default/server/types.ts
[error] 1-1: Missing license header.
⏰ 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: integration_prebuild
- GitHub Check: build_sdk
- GitHub Check: test_net
- GitHub Check: test_contracts
- GitHub Check: build_enclave_cli
- GitHub Check: rust_unit
- GitHub Check: crisp_rust_unit
- GitHub Check: rust_integration
- GitHub Check: build_e3_support_risc0
- GitHub Check: Build & Push Image
- GitHub Check: Build & Push Image
🔇 Additional comments (23)
templates/default/enclave.config.yaml (1)
6-6: No action required — address change is case normalization only.The e3_program address update is purely a case normalization to lowercase. Both addresses reference the same contract and have no functional impact.
templates/default/deployed_contracts.json (1)
70-76: Proxy address consistency verified—no action required.Cross-references between contracts are correct: Enclave references BondingRegistry (0x2279B7A0a67DB372996a5FaB50D91eAA73d2eBe6) and CiphernodeRegistryOwnable (0x610178dA211FEF7D417bC0e6FeD39F05609AD788) via their proxy addresses, and MyProgram references Enclave (0xA51c1fc2f0D1a1b8494Ed1FE312d7C3a78Ed91C0) via its proxy address. No implementation addresses are used in constructor arguments.
templates/default/package.json (1)
25-25: Acknowledge that 2.0.0-beta.12 is the only available version; clarify the actual concern is the "not audited" status.Line 25 adds
@zk-kit/lazy-imt.solat version2.0.0-beta.12. Web search confirms this is the only published version available (no stable release exists as of Mar 2024). However, the package is explicitly flagged as "not audited," which is the real concern for production merkle tree code. Before merging:
- Verify that the team is aware of and accepts the audit risk for this critical infrastructure.
- Consider whether @zk-kit/lean-imt.sol (which has stable audited releases like 2.0.1) is a suitable alternative.
- Confirm this version matches what's used in examples/CRISP.
templates/default/deploy/default.ts (1)
24-27: PoseidonT3 dependency guard looks correct; ensure deployment key matchesThe explicit
PoseidonT3lookup and hard failure is a good safeguard against deployingMyProgramwithout its Poseidon library linked. Just make sure the script that deploys PoseidonT3 callsstoreDeploymentArgswith the exact same contract name"PoseidonT3"and the samechainkey, otherwise this check will fail even though the library exists.examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol (8)
13-14: LGTM! LazyIMT infrastructure added correctly.The imports and using statement properly set up the InternalLazyIMT library for Merkle tree operations at the program level, aligning with the PR objective.
Also applies to: 17-18
20-27: LGTM! RoundData structure simplified.The removal of
snapshotBlocksimplifies the round data structure while retaining the essential census and governance parameters.
50-53: LGTM! Program-level vote storage structures added.The
voteSlotsandvotesmappings correctly implement per-E3 vote storage, enabling application-specific input policies as intended by issue #981.
75-76: LGTM! InputPublished event declared.The event properly captures the essential information (e3Id, vote content, tree index) needed to track program-level input publication.
83-95: LGTM! Constructor properly initializes dependencies.All critical components (enclave, verifiers, imageId) are validated and initialized correctly.
97-114: LGTM! setRoundData updated to match simplified RoundData struct.The function correctly initializes round data without the removed
snapshotBlockfield, maintaining the one-time setting constraint.
124-127: LGTM! setVerifier properly validates input.The zero-address check with custom error follows best practices.
197-217: LGTM with dependency! Verify function correctly uses program-level Merkle root.Line 208 properly retrieves the Merkle root from
votes[e3Id]._root()instead of calling the enclave, successfully decentralizing input storage as intended. However, this depends onvalidateInputcorrectly populating the tree (see critical issue above).templates/default/server/utils.ts (1)
20-20: LGTM!The addition of
E3_PROGRAM_ADDRESSfollows the established pattern and is correctly consumed by the server to listen to program-level events.packages/enclave-sdk/src/enclave-sdk.ts (2)
64-64: LGTM!The
publicClientfield is properly declared and initialized from the validated config.Also applies to: 103-103
124-130: LGTM!The
getPublicClientmethod provides clean access to the internalPublicClient, enabling direct contract event watching as required by the new architecture.templates/default/tests/integration.spec.ts (1)
255-266: LGTM!Removing the
INPUT_PUBLISHEDevent waits correctly reflects the architectural change where input tracking moved from the Enclave to the program level. The test still properly validates the computation result through thePLAINTEXT_OUTPUT_PUBLISHEDevent.examples/CRISP/server/src/server/repo.rs (4)
94-107: LGTM!The
insert_ciphertext_inputmethod correctly appends input data to the program-level storage using atomic modify operations with proper error handling.
124-124: LGTM!Properly initializes the
ciphertext_inputsfield as an empty vector during round initialization.
138-141: LGTM! Vote counting is now consistent.Both
get_vote_countandget_e3_state_litecorrectly derivevote_countfrome3_crisp.ciphertext_inputs, ensuring consistency across all vote-counting logic as intended by the architectural change.Also applies to: 208-208
218-221: LGTM!The
get_ciphertext_inputsmethod provides clean access to the program-level ciphertext inputs, supporting the new architecture where input storage is managed at the application layer.templates/default/server/types.ts (1)
3-13: LGTM!The event type definitions correctly model the program-level
InputPublishedevent structure and provide proper TypeScript typing for the new event-listening architecture.templates/default/server/index.ts (2)
14-14: LGTM!The new imports correctly support the transition to direct contract event watching via Viem's
PublicClient.Also applies to: 18-19
184-185: LGTM!The parameter type correctly updated to
InputPublishedEventto match the new program-level event schema.
|
❌ License Header Check Failed Some files are missing the required SPDX license header. Please add the following header to the beginning of all You can run Or run |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol (1)
202-205: Outdated comment about Enclave input Merkle treeThe comment:
// return the vote so that it can be stored in Enclave's input merkle treeno longer reflects the new architecture where the IMT lives at the program level and you emit
InputPublishedhere. If Enclave no longer stores an input Merkle tree, consider updating this comment (or removing it) to avoid confusion and documenting instead that the return value is kept for interface compatibility while indexers rely onInputPublished.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 996
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:144-169
Timestamp: 2025-11-12T10:08:30.720Z
Learning: In the CRISP program (examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol), there are plans to move the merkle tree from the Enclave contract to the program contract itself in the future, which would allow direct calls to validateInput to be stored in the merkle tree.
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.
📚 Learning: 2025-11-12T10:08:30.720Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 996
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:144-169
Timestamp: 2025-11-12T10:08:30.720Z
Learning: In the CRISP program (examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol), there are plans to move the merkle tree from the Enclave contract to the program contract itself in the future, which would allow direct calls to validateInput to be stored in the merkle tree.
Applied to files:
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build_enclave_cli
- GitHub Check: build_sdk
- GitHub Check: test_contracts
- GitHub Check: test_net
- GitHub Check: integration_prebuild
- GitHub Check: rust_integration
- GitHub Check: rust_unit
- GitHub Check: crisp_rust_unit
- GitHub Check: build_e3_support_risc0
- GitHub Check: Build & Push Image
- GitHub Check: Build & Push Image
🔇 Additional comments (5)
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol (5)
13-19: LazyIMT integration wiring looks goodImporting
LazyIMTData, InternalLazyIMTand attachingInternalLazyIMTtoLazyIMTDataat contract scope is clean and keeps Merkle logic nicely encapsulated. No issues here.
50-53: Per‑E3Id IMT state aligns with the new architecture
voteSlotsandvoteskeyed bye3Idgive you a clean per‑E3 input Merkle tree and avoid cross‑round contamination, which matches the PR objective of moving input storage to the program level.
75-76: Program‑levelInputPublishedevent is appropriateEmitting
InputPublished(e3Id, vote, index)from the program (instead of Enclave) is consistent with the new responsibility split and gives indexer/server enough data (e3Id + ciphertext + IMT index) to track inputs.
257-257: Using program‑level LazyIMT root inverifyis consistent with the designSwitching
inputRoottobytes32(votes[e3Id]._root())correctly wires proof verification to the program‑level IMT rather thanenclave.getInputRoot(e3Id), which matches the goal of moving input storage out of the Enclave.Please double‑check that all other components (Enclave contract, indexer, server) no longer rely on
getInputRootand instead use the program‑level events/roots.
150-151:validateInputis missingoverridekeyword, but the suggestedcalldatachange is incorrectThe missing
overridespecifier is a legitimate issue—Solidity 0.8.27 requires theoverridekeyword when implementing interface methods, and this will cause a compile-time error.However, the review's suggestion to use
bytes calldata datais incorrect. The interface declaration atpackages/enclave-contracts/contracts/interfaces/IE3Program.solspecifiesbytes memory data, notbytes calldata. Changing tocalldatawould violate the interface contract signature and create a parameter type mismatch compilation error instead of fixing it.The correct fix is to add only the
overridekeyword while keepingbytes memory data:/// @inheritdoc IE3Program - function validateInput(uint256 e3Id, address, bytes memory data) external returns (bytes memory input) { + function validateInput(uint256 e3Id, address, bytes memory data) external override returns (bytes memory input) {Likely an incorrect or invalid review comment.
|
❌ License Header Check Failed Some files are missing the required SPDX license header. Please add the following header to the beginning of all You can run Or run |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/compute-provider/Cargo.toml (1)
11-20: Remove unusedlean-imtdependency and fix version pinning for consistency.
lean-imt(line 11) is unused—no imports or references found in the codebase. The active IMT implementation iszk-kit-imtinmerkle_tree_builder.rs. Remove the unused dependency.Also,
zk-kit-imtshould be pinned with"="to match the pinning strategy used for all other dependencies in this file.Changes needed:
- Remove line 11:
lean-imt = "=0.1.2"- Change line 20:
zk-kit-imt = "0.0.7"tozk-kit-imt = "=0.0.7"examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol (1)
150-157: IMT state not populated in validateInput—verifywill read empty tree.The
voteSlotsandvotesmappings are declared (lines 50-53) butvalidateInputnever writes to them. The function decodes the vote but only returns it without inserting into the merkle tree. This causesverify(line 208) to read from an empty IMT, producing an incorrectinputRoot.Compare with
CRISPProgram.solwhich correctly calls_processVoteto insert/update the IMT. This mock should either:
- Mirror the main contract's behavior for realistic testing
- Be clearly documented as a simplified mock that skips IMT operations
If this mock needs to match the main contract's behavior, apply similar logic:
function validateInput(uint256 e3Id, address, bytes memory data) external returns (bytes memory input) { if (data.length == 0) revert EmptyInputData(); - (,, bytes memory vote,) = abi.decode(data, (bytes, bytes32[], bytes, address)); + (bytes memory noirProof, bytes memory vote, address slot) = abi.decode(data, (bytes, bytes, address)); + + uint40 storedIndexPlusOne = voteSlots[e3Id][slot]; + uint40 voteIndex; + + if (storedIndexPlusOne == 0) { + voteIndex = votes[e3Id].numberOfLeaves; + voteSlots[e3Id][slot] = voteIndex + 1; + votes[e3Id]._insert(uint256(keccak256(vote))); + } else { + voteIndex = storedIndexPlusOne - 1; + votes[e3Id]._update(uint256(keccak256(vote)), voteIndex); + } + + emit InputPublished(e3Id, vote, voteIndex); input = vote; }
♻️ Duplicate comments (5)
examples/CRISP/packages/crisp-contracts/deployed_contracts.json (1)
109-111: Unresolved: MockRISC0Verifier still missing blockNumber field.This issue was flagged in a previous review but remains unresolved. MockRISC0Verifier is the only contract in the sepolia section lacking the
blockNumberfield, breaking schema consistency with all other mock contracts (MockComputeProvider, MockDecryptionVerifier, MockE3Program). Update the deployment script to capture the block number when deploying MockRISC0Verifier.templates/default/deployed_contracts.json (1)
83-83: Fix placeholder enclave address in CiphernodeRegistryOwnable.Line 83 contains a zero-address placeholder (
0x0000000000000000000000000000000000000001) for theenclaveAddressconstructor argument. This should reference the Enclave proxy address deployed at line 110 (0xA51c1fc2f0D1a1b8494Ed1FE312d7C3a78Ed91C0).Apply this diff to correct the reference:
- "enclaveAddress": "0x0000000000000000000000000000000000000001", + "enclaveAddress": "0xA51c1fc2f0D1a1b8494Ed1FE312d7C3a78Ed91C0",packages/enclave-sdk/src/enclave-sdk.ts (1)
468-492: Critical:this.publicClientnot synchronized with config updates.After calling
updateConfig({ publicClient: newClient }),getPublicClient()will return the old client from the constructor becausethis.publicClientis never updated. Onlythis.config.publicClientgets reassigned (line 470).Apply this diff to keep
this.publicClientin sync:public updateConfig(newConfig: Partial<SDKConfig>): void { if (newConfig.publicClient) { this.config.publicClient = newConfig.publicClient + this.publicClient = newConfig.publicClient this.eventListener = new EventListener(newConfig.publicClient) }crates/compute-provider/src/merkle_tree_builder.rs (1)
38-55: Critical:Fr::from_strcan panic on arbitrary Keccak256 outputs.The BN254 field modulus is 254 bits, but Keccak256 produces 256-bit outputs. When the hash value exceeds the modulus,
Fr::from_strreturnsErrand.unwrap()panics.Use
Fr::from_be_bytes_mod_orderfor safe modular reduction:- let hex_output = hex::encode(Keccak256::digest(&item.0)); - let sanitized_hex = hex_output.trim_start_matches("0x"); - let numeric_value = BigUint::from_str_radix(sanitized_hex, 16) - .unwrap() - .to_string(); - let fr_element = Fr::from_str(&numeric_value).unwrap(); - let index_element = Fr::from_str(&item.1.to_string()).unwrap(); + let digest = Keccak256::digest(&item.0); + let fr_element = Fr::from_be_bytes_mod_order(&digest); + let index_element = Fr::from(item.1);This also applies to
poseidon_hash(lines 57-72).examples/CRISP/server/src/server/indexer.rs (1)
402-426: Use consistent logging.The
println!at lines 412-417 should use theinfo!macro for consistency with other event handlers in this file.Apply this diff:
- println!( + info!( "InputPublished: e3_id={}, index={}, data=0x{}...", event.e3Id, event.index, hex::encode(&event.vote[..8.min(event.vote.len())]) );
🧹 Nitpick comments (7)
crates/compute-provider/src/compute_input.rs (1)
37-41: Avoid cloningleaf_hashesif possible.The
.clone()on line 38 copies the entire vector of leaf hashes. Consider modifyingwith_leaf_hashesto accept ownership (move) directly, or pass by reference if the builder doesn't need to own the data.- let merkle_root = MerkleTreeBuilder::new() - .with_leaf_hashes(self.leaf_hashes.clone()) + let merkle_root = MerkleTreeBuilder::new() + .with_leaf_hashes(std::mem::take(&mut self.leaf_hashes))Note: This requires changing
processto take&mut selfor restructuring ownership. Alternatively, ifselfis not needed afterward, consider taking ownership ofComputeInput.crates/compute-provider/src/compute_manager.rs (1)
103-103: Avoid cloningleaf_hashes.Similar to
compute_input.rs, the clone here copies the vector. Consider taking ownership from the builder or usingstd::mem::takeif the builder is no longer needed.- leaf_hashes: tree_builder.leaf_hashes.clone(), + leaf_hashes: std::mem::take(&mut tree_builder.leaf_hashes),crates/compute-provider/src/merkle_tree_builder.rs (2)
23-31: Consider implementingDefaulttrait.Since
new()provides sensible defaults, implementingDefaultwould be more idiomatic and enable use with..Default::default()patterns.+impl Default for MerkleTreeBuilder { + fn default() -> Self { + Self::new() + } +} + impl MerkleTreeBuilder { pub fn new() -> Self {
74-89: Add error context to.unwrap()calls or propagate errors.The multiple
.unwrap()calls on lines 82 and 85 will produce unhelpful panic messages if they fail. Consider using.expect()with descriptive messages or converting toResultreturn type.- let mut tree = IMT::new( + let mut tree = IMT::new( Self::poseidon_hash, self.depth, self.zero_value.clone(), self.arity, vec![], ) - .unwrap(); + .expect("Failed to initialize IMT with given parameters"); for leaf in &self.leaf_hashes { - tree.insert(leaf.clone()).unwrap(); + tree.insert(leaf.clone()).expect("Failed to insert leaf into IMT"); }examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol (2)
155-169: State modified before proof verification—consider reordering.
_processVote(line 155) modifies storage beforeHONK_VERIFIER.verify(line 167). While the transaction reverts on failure, this wastes gas on storage writes if verification fails.Since the Noir circuit only validates the slot address and isFirstVote flag (neither depends on post-state), you can safely move verification before state modification:
(bytes memory noirProof, bytes memory vote, address slot) = abi.decode(data, (bytes, bytes, address)); - (uint40 voteIndex, bool isFirstVote) = _processVote(e3Id, slot, vote); + // Compute isFirstVote before state modification + uint40 storedIndexPlusOne = voteSlots[e3Id][slot]; + bool isFirstVote = storedIndexPlusOne == 0; bytes32[] memory noirPublicInputs = new bytes32[](2); // Set public inputs for the proof. Order must match Noir circuit. noirPublicInputs[0] = bytes32(uint256(uint160(slot))); // Pass isFirstVote flag to verifier (1 = first vote, 0 = re-vote) noirPublicInputs[1] = bytes32(uint256(isFirstVote ? 1 : 0)); - // noirPublicInputs[x] = bytes32(roundData.censusMerkleRoot); - // Check if the ciphertext was encrypted correctly if (!HONK_VERIFIER.verify(noirProof, noirPublicInputs)) { revert InvalidNoirProof(); } + // Modify state only after successful verification + (uint40 voteIndex, ) = _processVote(e3Id, slot, vote); + // return the vote so that it can be stored in Enclave's input merkle tree input = vote; emit InputPublished(e3Id, vote, voteIndex);
164-164: Remove commented-out dead code.This commented line references a non-existent
roundData.censusMerkleRoot(should beroundsData[e3Id].censusMerkleRoot) and the Noir circuit'smerkle_rootis a private input, not public. Remove to avoid confusion.- // noirPublicInputs[x] = bytes32(roundData.censusMerkleRoot);templates/default/server/index.ts (1)
186-204: Unsafe type assertion bypasses viem's Log structure.The cast at line 199 (
log as unknown as RawInputPublishedEvent) is unsafe. Viem'swatchContractEventreturnsLogobjects where decoded event parameters are inlog.args, butRawInputPublishedEventexpects{ eventName: string, args: InputPublishedEvent }.The current cast may work at runtime due to how viem structures its logs, but it bypasses type validation. Consider explicitly constructing the event:
async onLogs(logs: Log[]) { for (let i = 0; i < logs.length; i++) { const log = logs[i] if (!log) { console.log('warning: Log was falsy when a log was expected!') break } - const eventData = log as unknown as RawInputPublishedEvent + if (!('args' in log) || !log.args) { + console.log('warning: Log missing decoded args') + continue + } + const eventData: RawInputPublishedEvent = { + eventName: ProgramEventType.INPUT_PUBLISHED, + args: log.args as InputPublishedEvent + } await handleInputPublishedEvent(eventData) } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.lockcrates/support/Cargo.lockis excluded by!**/*.lockcrates/support/methods/guest/Cargo.lockis excluded by!**/*.lockexamples/CRISP/Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yamltemplates/default/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (56)
.github/workflows/ci.yml(1 hunks)crates/compute-provider/Cargo.toml(1 hunks)crates/compute-provider/src/compute_input.rs(2 hunks)crates/compute-provider/src/compute_manager.rs(4 hunks)crates/compute-provider/src/lib.rs(1 hunks)crates/compute-provider/src/merkle_tree_builder.rs(2 hunks)crates/evm-helpers/src/events.rs(0 hunks)crates/indexer/src/indexer.rs(1 hunks)crates/indexer/tests/integration.rs(0 hunks)crates/support/Cargo.toml(2 hunks)crates/support/contracts/ImageID.sol(1 hunks)crates/support/host/Cargo.toml(1 hunks)crates/support/methods/guest/Cargo.toml(1 hunks)crates/support/program/Cargo.toml(1 hunks)eslint.config.mjs(1 hunks)examples/CRISP/client/libs/wasm/pkg/crisp_worker.js(1 hunks)examples/CRISP/crates/evm_helpers/src/lib.rs(2 hunks)examples/CRISP/eslint.config.js(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol(7 hunks)examples/CRISP/packages/crisp-contracts/deploy/crisp.ts(3 hunks)examples/CRISP/packages/crisp-contracts/deployed_contracts.json(2 hunks)examples/CRISP/packages/crisp-contracts/package.json(1 hunks)examples/CRISP/packages/crisp-contracts/remappings.txt(1 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(2 hunks)examples/CRISP/server/src/server/indexer.rs(7 hunks)examples/CRISP/server/src/server/mod.rs(1 hunks)examples/CRISP/server/src/server/models.rs(1 hunks)examples/CRISP/server/src/server/repo.rs(5 hunks)package.json(1 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.json(1 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json(1 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json(1 hunks)packages/enclave-contracts/contracts/Enclave.sol(1 hunks)packages/enclave-contracts/contracts/interfaces/IE3Program.sol(1 hunks)packages/enclave-contracts/contracts/interfaces/IEnclave.sol(0 hunks)packages/enclave-contracts/contracts/test/MockE3Program.sol(1 hunks)packages/enclave-contracts/ignition/modules/enclave.ts(1 hunks)packages/enclave-contracts/package.json(2 hunks)packages/enclave-contracts/scripts/deployAndSave/enclave.ts(2 hunks)packages/enclave-contracts/scripts/deployEnclave.ts(0 hunks)packages/enclave-contracts/scripts/upgrade/enclave.ts(0 hunks)packages/enclave-contracts/tasks/enclave.ts(0 hunks)packages/enclave-contracts/test/Enclave.spec.ts(0 hunks)packages/enclave-react/src/index.ts(0 hunks)packages/enclave-sdk/src/enclave-sdk.ts(4 hunks)packages/enclave-sdk/src/index.ts(0 hunks)packages/enclave-sdk/src/types.ts(0 hunks)templates/default/contracts/MyProgram.sol(7 hunks)templates/default/deploy/default.ts(3 hunks)templates/default/deployed_contracts.json(2 hunks)templates/default/package.json(1 hunks)templates/default/server/index.ts(3 hunks)templates/default/server/types.ts(1 hunks)templates/default/server/utils.ts(1 hunks)templates/default/tests/integration.spec.ts(1 hunks)
💤 Files with no reviewable changes (10)
- crates/indexer/tests/integration.rs
- packages/enclave-sdk/src/types.ts
- packages/enclave-contracts/test/Enclave.spec.ts
- packages/enclave-contracts/scripts/upgrade/enclave.ts
- packages/enclave-contracts/tasks/enclave.ts
- packages/enclave-sdk/src/index.ts
- packages/enclave-contracts/contracts/interfaces/IEnclave.sol
- packages/enclave-contracts/scripts/deployEnclave.ts
- crates/evm-helpers/src/events.rs
- packages/enclave-react/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (25)
- .github/workflows/ci.yml
- templates/default/package.json
- crates/indexer/src/indexer.rs
- examples/CRISP/packages/crisp-contracts/remappings.txt
- examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts
- examples/CRISP/packages/crisp-contracts/package.json
- package.json
- templates/default/tests/integration.spec.ts
- examples/CRISP/server/src/server/repo.rs
- examples/CRISP/server/src/server/models.rs
- packages/enclave-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.json
- packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json
- examples/CRISP/server/src/server/mod.rs
- packages/enclave-contracts/package.json
- templates/default/server/types.ts
- crates/compute-provider/src/lib.rs
- examples/CRISP/packages/crisp-contracts/deploy/crisp.ts
- eslint.config.mjs
- examples/CRISP/eslint.config.js
- crates/support/host/Cargo.toml
- crates/support/Cargo.toml
- examples/CRISP/client/libs/wasm/pkg/crisp_worker.js
- crates/support/methods/guest/Cargo.toml
- templates/default/server/utils.ts
- crates/support/program/Cargo.toml
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 996
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:144-169
Timestamp: 2025-11-12T10:08:30.720Z
Learning: In the CRISP program (examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol), there are plans to move the merkle tree from the Enclave contract to the program contract itself in the future, which would allow direct calls to validateInput to be stored in the merkle tree.
📚 Learning: 2025-11-12T10:08:30.720Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 996
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:144-169
Timestamp: 2025-11-12T10:08:30.720Z
Learning: In the CRISP program (examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol), there are plans to move the merkle tree from the Enclave contract to the program contract itself in the future, which would allow direct calls to validateInput to be stored in the merkle tree.
Applied to files:
templates/default/contracts/MyProgram.solexamples/CRISP/crates/evm_helpers/src/lib.rscrates/compute-provider/src/compute_input.rspackages/enclave-contracts/contracts/Enclave.solexamples/CRISP/server/src/server/indexer.rsexamples/CRISP/packages/crisp-contracts/deployed_contracts.jsonpackages/enclave-contracts/contracts/test/MockE3Program.solexamples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.solpackages/enclave-contracts/contracts/interfaces/IE3Program.solexamples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol
📚 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:
crates/compute-provider/Cargo.toml
📚 Learning: 2024-09-26T03:11:29.311Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-09-26T03:11:29.311Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Applied to files:
examples/CRISP/crates/evm_helpers/src/lib.rscrates/compute-provider/src/compute_manager.rs
📚 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:
packages/enclave-contracts/contracts/Enclave.solpackages/enclave-contracts/ignition/modules/enclave.tspackages/enclave-contracts/scripts/deployAndSave/enclave.tstemplates/default/deployed_contracts.json
📚 Learning: 2024-11-05T06:49:46.285Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 173
File: packages/ciphernode/enclave_node/src/datastore.rs:14-16
Timestamp: 2024-11-05T06:49:46.285Z
Learning: In `packages/ciphernode/enclave_node/src/datastore.rs`, for internal functions like `get_in_mem_store`, adding extensive documentation and error handling may not be necessary if they are not client-facing.
Applied to files:
examples/CRISP/server/src/server/indexer.rs
📚 Learning: 2024-10-28T12:00:09.010Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/enclave/src/commands/wallet/set.rs:17-23
Timestamp: 2024-10-28T12:00:09.010Z
Learning: In the `enclave` package of the `ciphernode` project, prefer using `println!` over logging macros like `error!` from the `tracing` crate for error output in CLI commands.
Applied to files:
examples/CRISP/server/src/server/indexer.rs
📚 Learning: 2024-10-10T23:24:43.341Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 143
File: packages/ciphernode/sortition/src/sortition.rs:4-9
Timestamp: 2024-10-10T23:24:43.341Z
Learning: In the `Sortition` module (`packages/ciphernode/sortition/src/sortition.rs`), errors are sent to the event bus using `self.bus.err`, which handles logging and printing. Therefore, explicit use of the `tracing` crate for logging errors may not be necessary in this context.
Applied to files:
examples/CRISP/server/src/server/indexer.rs
📚 Learning: 2024-10-10T11:26:47.259Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 143
File: packages/ciphernode/p2p/src/libp2p_router.rs:154-154
Timestamp: 2024-10-10T11:26:47.259Z
Learning: In `packages/ciphernode/p2p/src/libp2p_router.rs`, logging the entire message data using `trace!("{:?}", message);` is acceptable at this point.
Applied to files:
examples/CRISP/server/src/server/indexer.rs
📚 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:
crates/compute-provider/src/merkle_tree_builder.rs
📚 Learning: 2024-10-22T03:39:29.448Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/evm/src/enclave_sol_reader.rs:87-89
Timestamp: 2024-10-22T03:39:29.448Z
Learning: In the `ciphernode` project, specifically in `packages/ciphernode/evm/src/enclave_sol_reader.rs`, the method `EnclaveSolReader::attach` should be retained even if it directly calls `EvmEventReader::attach` without additional processing. Avoid suggesting its removal in future reviews.
Applied to files:
crates/compute-provider/src/merkle_tree_builder.rstemplates/default/deployed_contracts.json
📚 Learning: 2025-08-28T08:58:17.434Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 665
File: packages/enclave-sdk/src/contract-client.ts:110-118
Timestamp: 2025-08-28T08:58:17.434Z
Learning: viem's TypeScript ABI inference maps uint32[2] to [number, number], not [bigint, bigint]. The library uses number type for uint32 values to align with traditional database patterns and avoid unnecessary BigInt conversion.
Applied to files:
packages/enclave-sdk/src/enclave-sdk.ts
📚 Learning: 2025-09-11T13:02:56.353Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts:6-6
Timestamp: 2025-09-11T13:02:56.353Z
Learning: In Hardhat v3, the import path "hardhat/types/hre" may be valid for importing HardhatRuntimeEnvironment, unlike in Hardhat v2 where the standard path was "hardhat/types".
Applied to files:
packages/enclave-contracts/scripts/deployAndSave/enclave.ts
📚 Learning: 2025-09-11T13:02:56.353Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts:6-6
Timestamp: 2025-09-11T13:02:56.353Z
Learning: In Hardhat v3, the import path "hardhat/types/hre" is valid for importing HardhatRuntimeEnvironment, which is different from Hardhat v2 where "hardhat/types" was the standard path.
Applied to files:
packages/enclave-contracts/scripts/deployAndSave/enclave.ts
📚 Learning: 2025-09-11T13:02:56.353Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts:6-6
Timestamp: 2025-09-11T13:02:56.353Z
Learning: In Hardhat v3, the correct import path for HardhatRuntimeEnvironment is "hardhat/types/hre", not "hardhat/types" as it was in Hardhat v2. The import structure has changed between major versions.
Applied to files:
packages/enclave-contracts/scripts/deployAndSave/enclave.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, hre.network.name is not available anymore. Use hre.globalOptions.network to get the network name instead.
Applied to files:
packages/enclave-contracts/scripts/deployAndSave/enclave.ts
📚 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:
templates/default/deployed_contracts.json
🧬 Code graph analysis (6)
examples/CRISP/crates/evm_helpers/src/lib.rs (1)
examples/CRISP/server/src/server/indexer.rs (8)
event(48-48)event(98-98)event(190-190)event(193-193)event(280-280)event(298-298)event(409-409)event(419-419)
crates/compute-provider/src/compute_input.rs (2)
crates/compute-provider/src/compute_manager.rs (1)
new(30-48)crates/compute-provider/src/merkle_tree_builder.rs (1)
new(24-31)
templates/default/deploy/default.ts (1)
packages/enclave-contracts/scripts/utils.ts (1)
readDeploymentArgs(96-110)
templates/default/server/index.ts (2)
templates/default/server/types.ts (1)
RawInputPublishedEvent(21-24)templates/default/server/utils.ts (1)
getCheckedEnvVars(15-27)
crates/compute-provider/src/compute_manager.rs (1)
crates/compute-provider/src/merkle_tree_builder.rs (1)
new(24-31)
examples/CRISP/server/src/server/indexer.rs (2)
crates/indexer/src/indexer.rs (4)
store(356-359)new(44-48)new(113-115)new(175-191)crates/evm-helpers/src/listener.rs (2)
contract_address(99-99)create_contract_listener(97-104)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: test_contracts
- GitHub Check: build_enclave_cli
- GitHub Check: integration_prebuild
- GitHub Check: build_sdk
- GitHub Check: Build & Push Image
- GitHub Check: test_net
- GitHub Check: rust_integration
- GitHub Check: crisp_rust_unit
- GitHub Check: rust_unit
🔇 Additional comments (34)
packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json (1)
538-538: No code review needed for build artifact.This is an auto-generated Hardhat artifact file. The
buildInfoIdhash update reflects recompilation triggered by changes elsewhere in the PR. The ABI and contract interface remain unchanged.crates/support/contracts/ImageID.sol (1)
22-22: LGTM! Expected update to auto-generated program ID.The
PROGRAM_IDvalue has been updated as expected. Since this file is auto-generated and the PR changes the program logic (moving Merkle tree handling to the program level), the compiled program's image ID naturally changes. This new identifier will be used to verify that the correct program version is executed in the zkVM.templates/default/deployed_contracts.json (1)
70-115: Verify proxyRecords alignment across contracts.The new
proxyRecordsblocks correctly align top-leveladdressfields with their correspondingproxyAddressvalues for BondingRegistry, CiphernodeRegistryOwnable, and Enclave. The restructuring looks consistent and sound.packages/enclave-sdk/src/enclave-sdk.ts (4)
7-7: LGTM! Import addition is correct.The
PublicClienttype import is properly added to support the new field and getter method.
51-51: LGTM! Field declaration is appropriate.The private field is correctly typed and positioned with other private instance fields.
84-84: LGTM! Constructor assignment is correct.The field is properly initialized after validation, ensuring
publicClientis available for the getter.
102-108: LGTM! Getter implementation is clean.The method correctly exposes the stored
PublicClientinstance with appropriate documentation.crates/compute-provider/src/compute_manager.rs (1)
59-62: LGTM!The sequential path correctly uses
MerkleTreeBuilder::new()which now initializes with sensible defaults (arity=2,depth=20). The builder pattern is clean and the flow is clear.packages/enclave-contracts/ignition/modules/enclave.ts (1)
16-16: LGTM! Correct removal of PoseidonT3 library binding.This aligns with the PR's objective of moving merkle tree management from Enclave to the program level. The Enclave contract no longer needs the PoseidonT3 library since each program now manages its own IMT.
packages/enclave-contracts/scripts/deployAndSave/enclave.ts (2)
77-77: LGTM! Factory creation simplified correctly.The removal of PoseidonT3 library linking from the Enclave deployment is consistent with the architectural change moving merkle tree management to the program level.
160-160: Upgrade path correctly aligned with deploy path.Both
deployAndSaveEnclaveandupgradeAndSaveEnclavenow use the simplified factory creation without PoseidonT3 linking, maintaining consistency across deployment workflows.examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol (4)
8-18: LGTM! Well-structured imports and LazyIMT integration.The per-e3 data model using
InternalLazyIMTwithPoseidonT3is correctly set up. Theusingdirective properly enables the library functions onLazyIMTData.
41-56: Per-e3 state model correctly addresses Issue #981 and #1037.The per-e3 mappings (
roundsData,isRoundsDataSet,voteSlots,votes) properly scope state to individual e3 instances, fixing the flag scope issue and enabling program-specific input handling. Based on learnings, this fulfills the planned move of merkle tree from Enclave to program level.
179-196: Correct implementation of sentinel pattern for vote slot tracking.The
index + 1sentinel pattern correctly distinguishes "no previous vote" (0) from "voted at index 0" (stored as 1). This addresses the critical bug from previous reviews where the ambiguous use of 0 caused incorrect insert/update branching.
240-251: LGTM! Verify correctly uses per-e3 IMT with fixed depth.The
_root(TREE_DEPTH)call ensures consistent root computation matching the compute provider's expectations. The per-e3paramsHashescheck and journal encoding are correctly implemented.templates/default/deploy/default.ts (2)
24-27: Good validation of PoseidonT3 dependency.Explicitly checking for and requiring the PoseidonT3 address before deployment prevents cryptic failures during contract deployment. This follows the pattern established in other deployment scripts.
45-53: Correct factory-based deployment with library linking.The
linkBytecodeapproach with the proper solc linking path format ('npm/poseidon-solidity@0.0.5/PoseidonT3.sol:PoseidonT3') correctly links the PoseidonT3 library at deployment time.packages/enclave-contracts/contracts/test/MockE3Program.sol (1)
36-46: LGTM!The
validateInputsignature correctly aligns with the updatedIE3Programinterface. Using an unnamed first parameter (uint256,) is appropriate since this mock doesn't need to use thee3Idfor its validation logic. Based on learnings, this change supports the planned move of the Merkle tree from Enclave to the program contract.packages/enclave-contracts/contracts/interfaces/IE3Program.sol (1)
40-50: LGTM!The addition of
e3Idparameter tovalidateInputenables programs to maintain per-E3 state (such as Merkle trees), which aligns with the PR objective of moving input management from Enclave to the program layer. The NatSpec documentation is properly updated to describe the new parameter.templates/default/contracts/MyProgram.sol (5)
12-15: LGTM!Clean integration of LazyIMT for per-E3 Merkle tree management. The
usingdirective appropriately enables the library methods onLazyIMTData.
19-29: LGTM!The per-E3 inputs mapping with
LazyIMTDatacorrectly supports independent Merkle trees for each E3 computation.TREE_DEPTH = 20provides capacity for 2^20 (~1M) inputs per E3.
74-75: LGTM!Proper initialization of the per-E3 Merkle tree during E3 validation. This ensures each E3 has its own independent input tree.
83-99: LGTM!The
validateInputimplementation correctly:
- Retrieves the current leaf count as the index
- Inserts a Poseidon hash of
(keccak256(data), index)into the tree- Emits the
InputPublishedevent with all necessary data for off-chain indexingThis pattern enables program-specific input handling while maintaining cryptographic commitments.
105-120: LGTM!The
verifyfunction correctly derivesinputRootfrom the per-E3 tree (inputs[e3Id]._root()) instead of calling the removedenclave.getInputRoot(e3Id). This completes the migration of Merkle tree ownership to the program layer.packages/enclave-contracts/contracts/Enclave.sol (1)
323-341: LGTM!The
publishInputfunction is correctly simplified to delegate input validation entirely to the program layer viae3.e3Program.validateInput(e3Id, msg.sender, data). The program now handles Merkle tree insertion and event emission internally, which aligns with the PR objective of moving input management to the program level. Based on learnings, this enables program-specific input handling (e.g., CRISP considering only the latest vote per user).examples/CRISP/crates/evm_helpers/src/lib.rs (2)
24-34: LGTM!The Solidity bindings correctly reflect the contract changes:
setRoundDatanow includese3_idas the first parameter for per-E3 round data management- The
InputPublishedevent declaration enables the Rust indexer to decode these events from the CRISP program
83-100: LGTM!The
set_round_datamethod signature and contract call are correctly updated to passe3_idas the first argument, aligning with the per-E3 data model inCRISPProgram.sol.templates/default/server/index.ts (3)
8-14: LGTM!Appropriate imports added for the new event listening approach. Using
MyProgram__factory.abiensures type-safe event decoding with the correct ABI.
163-184: LGTM!The
handleInputPublishedEventcorrectly accesses the event data throughevent.args.*properties, aligning with theRawInputPublishedEventinterface that wraps the decoded event arguments.
206-217: LGTM!The setup correctly retrieves
E3_PROGRAM_ADDRESSand wires up theInputPublishedevent listener on the program contract rather than the enclave contract, reflecting the architectural change where programs now emit these events.examples/CRISP/server/src/server/indexer.rs (4)
30-32: LGTM!Correct imports added for
SharedStore(to share the data store across handlers) andInputPublishedevent from the CRISP evm_helpers crate.
147-166: LGTM!The
e3_idis correctly converted toU256and passed as the first argument toset_round_data, aligning with the updatedCRISPProgram.setRoundDatasignature that now accepts per-E3 identifiers.
227-232: LGTM!The code correctly retrieves ciphertext inputs via
get_ciphertext_inputs()and passes them torun_compute. This reflects the shift to per-E3 input storage in the repository.
428-465: LGTM!The
start_indexerfunction is correctly updated to:
- Accept
crisp_addressfor the CRISP program contract- Pass the cloned
storetoEnclaveIndexer::new- Create a dedicated CRISP contract listener for
InputPublishedevents- Wire up
register_input_publishedwith the shared storeThis properly routes
InputPublishedevents from the CRISP program (not Enclave) to the indexer for persistence.
5 participants
Closes #981
Closes #947
Fixes #1037
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.