feat: do not use external input validator for programs [skip-line-limit]#996
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughConsolidates input validation into the E3Program: removes the IInputValidator interface and implementations, updates IE3Program.validate() to return only bytes32, and adds validateInput(address, bytes) for program-owned input processing; Enclave and tests updated to call validateInput. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Enclave
participant E3Program
participant InputValidator as OldValidator
rect rgba(220,220,255,0.15)
Note over User,OldValidator: Old flow (separate validator)
User->>Enclave: request(e3Id, seed, params)
Enclave->>E3Program: validate(e3Id, seed, ...)
E3Program-->>Enclave: (encryptionSchemeId, inputValidator)
User->>Enclave: publishInput(data)
Enclave->>OldValidator: validate(msg.sender, data)
OldValidator-->>Enclave: input
end
rect rgba(220,255,220,0.15)
Note over User,E3Program: New flow (program-owned validation)
User->>Enclave: request(e3Id, seed, params)
Enclave->>E3Program: validate(e3Id, seed, ...)
E3Program-->>Enclave: encryptionSchemeId
User->>Enclave: publishInput(data)
Enclave->>E3Program: validateInput(msg.sender, data)
E3Program-->>Enclave: input
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (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). (9)
🔇 Additional comments (1)
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 |
ctrlc03
force-pushed
the
feat/no-input-validator-external
branch
from
193fcae to
478512e
Compare
ctrlc03
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/pages/putting-it-together.mdx (1)
106-131: Document the newvalidateInputrequirementThe sample still only calls out
validate/verify, but the updatedIE3Programinterface now also requiresvalidateInput. Anyone copying this snippet will end up with a contract that fails to compile. Please update the prose and example to mention and stub outvalidateInputalongsidevalidateandverify.docs/pages/building-with-enclave.mdx (1)
120-128: Fix thevalidateInputsignature
IE3Program.validateInputnow returns a singlebytespayload—there’s nobool success. The signature shown here will not compile against the interface. Please update the code block (and surrounding text) to reflectfunction validateInput(address sender, bytes memory params) external returns (bytes memory input);.docs/pages/write-e3-contract.mdx (1)
13-44: Update architecture description to remove the external Input ValidatorThis section still tells readers that every E3 comprises “E3 Program + Input Validator” and even suggests passing an Input Validator address in
e3ProgramParams. With this PR, validation is now handled inside the Program viavalidateInput, so the guidance is wrong. Please rewrite these paragraphs to describe the single-program flow and remove the leftover references to a dedicated Input Validator contract.templates/default/contracts/MyProgram.sol (1)
55-84: Add the missingoverridespecifiersBoth
validateandvalidateInputimplementIE3Program, but withoutoverridethe compiler throwsTypeError: Overriding function missing 'override' specifier. Please restore the overrides.- ) external returns (bytes32) { + ) external override returns (bytes32) { ... - ) external returns (bytes memory input) { + ) external override returns (bytes memory input) {
🧹 Nitpick comments (1)
packages/enclave-contracts/tasks/enclave.ts (1)
165-168: Consider removing or standardizing debug logs for production.The debug console.log statements are helpful during development. Consider whether these should remain in production code or be replaced with a proper logging mechanism for consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (60)
README.md(1 hunks)crates/evm-helpers/src/contracts.rs(0 hunks)crates/evm-helpers/src/events.rs(0 hunks)crates/evm-helpers/tests/fixtures/fake_enclave.sol(0 hunks)crates/indexer/tests/fixtures/fake_enclave.sol(0 hunks)docs/pages/CRISP/introduction.mdx(1 hunks)docs/pages/CRISP/setup.mdx(0 hunks)docs/pages/architecture-overview.mdx(1 hunks)docs/pages/building-with-enclave.mdx(3 hunks)docs/pages/computation-flow.mdx(1 hunks)docs/pages/putting-it-together.mdx(1 hunks)docs/pages/write-e3-contract.mdx(3 hunks)examples/CRISP/Readme.md(0 hunks)examples/CRISP/enclave.config.yaml(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidator.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidatorFactory.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol(3 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol(16 hunks)examples/CRISP/packages/crisp-contracts/contracts/ImageID.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPInputValidator.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockRISC0Verifier.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/RiscZeroGroth16Verifier.sol(1 hunks)examples/CRISP/packages/crisp-contracts/deploy/crisp.ts(4 hunks)examples/CRISP/packages/crisp-contracts/deployed_contracts.json(1 hunks)examples/CRISP/packages/crisp-contracts/hardhat.config.ts(0 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(1 hunks)examples/CRISP/server/.env.example(1 hunks)examples/CRISP/server/src/server/indexer.rs(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(2 hunks)packages/enclave-contracts/contracts/interfaces/IE3.sol(0 hunks)packages/enclave-contracts/contracts/interfaces/IE3Program.sol(2 hunks)packages/enclave-contracts/contracts/interfaces/IInputValidator.sol(0 hunks)packages/enclave-contracts/contracts/lib/ExitQueueLib.sol(2 hunks)packages/enclave-contracts/contracts/registry/BondingRegistry.sol(2 hunks)packages/enclave-contracts/contracts/test/MockComputeProvider.sol(1 hunks)packages/enclave-contracts/contracts/test/MockE3Program.sol(1 hunks)packages/enclave-contracts/contracts/test/MockInputValidator.sol(0 hunks)packages/enclave-contracts/contracts/token/EnclaveToken.sol(2 hunks)packages/enclave-contracts/deployed_contracts.json(4 hunks)packages/enclave-contracts/ignition/modules/mockE3Program.ts(1 hunks)packages/enclave-contracts/ignition/modules/mockInputValidator.ts(0 hunks)packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts(0 hunks)packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts(2 hunks)packages/enclave-contracts/scripts/deployMocks.ts(0 hunks)packages/enclave-contracts/scripts/index.ts(0 hunks)packages/enclave-contracts/tasks/enclave.ts(2 hunks)packages/enclave-contracts/test/Enclave.spec.ts(3 hunks)packages/enclave-sdk/src/types.ts(0 hunks)templates/default/contracts/InputValidator.sol(0 hunks)templates/default/contracts/MyProgram.sol(2 hunks)templates/default/deploy/default.ts(1 hunks)templates/default/deployed_contracts.json(1 hunks)templates/default/enclave.config.yaml(1 hunks)templates/default/hardhat.config.ts(0 hunks)tests/integration/enclave.config.yaml(1 hunks)
💤 Files with no reviewable changes (21)
- packages/enclave-contracts/scripts/index.ts
- docs/pages/CRISP/setup.mdx
- packages/enclave-contracts/scripts/deployMocks.ts
- crates/evm-helpers/src/contracts.rs
- packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts
- crates/indexer/tests/fixtures/fake_enclave.sol
- packages/enclave-contracts/ignition/modules/mockInputValidator.ts
- examples/CRISP/Readme.md
- packages/enclave-contracts/contracts/interfaces/IInputValidator.sol
- examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol
- crates/evm-helpers/src/events.rs
- templates/default/hardhat.config.ts
- crates/evm-helpers/tests/fixtures/fake_enclave.sol
- examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidatorFactory.sol
- packages/enclave-contracts/contracts/test/MockInputValidator.sol
- examples/CRISP/packages/crisp-contracts/hardhat.config.ts
- templates/default/contracts/InputValidator.sol
- packages/enclave-contracts/contracts/interfaces/IE3.sol
- packages/enclave-sdk/src/types.ts
- examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPInputValidator.sol
- examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidator.sol
🧰 Additional context used
🧠 Learnings (6)
📚 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: 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/deployed_contracts.jsontests/integration/enclave.config.yamltemplates/default/enclave.config.yamlpackages/enclave-contracts/tasks/enclave.tsexamples/CRISP/enclave.config.yamlpackages/enclave-contracts/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: 2025-11-05T14:12:57.814Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 963
File: examples/CRISP/client/package.json:25-25
Timestamp: 2025-11-05T14:12:57.814Z
Learning: In the Enclave/CRISP codebase, `enclave-e3/sdk` and `crisp-e3/sdk` are different packages: `enclave-e3/sdk` is the general Enclave SDK, while `crisp-e3/sdk` is the CRISP-specific SDK. The CRISP client (`examples/CRISP/client`) intentionally depends on `enclave-e3/sdk`, not `crisp-e3/sdk`.
Applied to files:
examples/CRISP/packages/crisp-contracts/deploy/crisp.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/deploy/crisp.tspackages/enclave-contracts/tasks/enclave.tspackages/enclave-contracts/scripts/deployAndSave/mockProgram.ts
📚 Learning: 2025-11-08T01:31:47.484Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 969
File: packages/enclave-sdk/src/enclave-sdk.ts:101-103
Timestamp: 2025-11-08T01:31:47.484Z
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-contracts/tasks/enclave.ts
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (1)
packages/enclave-contracts/scripts/utils.ts (2)
storeDeploymentArgs(35-67)readDeploymentArgs(75-89)
🪛 dotenv-linter (4.0.0)
examples/CRISP/server/.env.example
[warning] 16-16: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 17-17: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 17-17: [UnorderedKey] The CIPHERNODE_REGISTRY_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
[warning] 18-18: [UnorderedKey] The E3_PROGRAM_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
🪛 Gitleaks (8.29.0)
templates/default/deployed_contracts.json
[high] 89-89: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
docs/pages/write-e3-contract.mdx
[style] ~67-~67: Consider a more concise word here.
Context: ...PRisc0.sol#L45). ### Input Validation In order to ensure correct computation, we should b...
(IN_ORDER_TO_PREMIUM)
🔇 Additional comments (30)
examples/CRISP/packages/crisp-contracts/contracts/Mocks/RiscZeroGroth16Verifier.sol (1)
12-12: Formatting change is fine.The constructor invocation has been reformatted to a single line while preserving the same arguments and behavior.
examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockRISC0Verifier.sol (1)
11-11: Formatting-only change to mock contract.The
verifyfunction signature is reformatted to a single line with no changes to parameters, visibility, or behavior. This is consistent with the PR's refactoring scope and the contract remains a valid mock implementation ofIRiscZeroVerifier.packages/enclave-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.json (1)
854-854: Build artifact rebuild with no ABI changes.The
buildInfoIdhas been updated due to artifact rebuild, but the ABI remains unchanged. All function signatures, events, and error types are identical to the previous version. This is a routine metadata update and does not affect contract functionality.examples/CRISP/packages/crisp-contracts/contracts/ImageID.sol (1)
22-22: Formatting change in auto-generated file — LGTM.The reformatting of
PROGRAM_IDfrom a multiline to single-line expression is a non-functional change. Since this file is auto-generated (per the header comment on line 17), the change reflects regeneration with an updated output format, which is expected as part of this broader refactoring effort.packages/enclave-contracts/contracts/registry/BondingRegistry.sol (2)
131-133: Formatting: Add braces for consistency.Wrapping the revert statement with braces improves consistency across the codebase by making single-statement if blocks explicit.
442-445: Formatting: Add braces for consistency.Wrapping the safeTransfer call with braces maintains consistent brace usage across single-statement conditional blocks.
packages/enclave-contracts/contracts/token/EnclaveToken.sol (2)
152-154: Formatting improvement: added braces for clarity.Block braces around the revert condition improve readability and consistency with Solidity best practices for control flow structures.
236-238: Formatting improvement: added braces for clarity.Block braces around the revert condition improve readability and maintain a consistent style with ERC20 override patterns.
packages/enclave-contracts/contracts/lib/ExitQueueLib.sol (1)
379-392: Consistent and well-formatted conditional braces in _updatePendingTotals.The addition of braces around all nested conditional bodies improves code clarity and consistency. This aligns with best practices for defensive coding.
packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json (1)
538-538: No action required; the buildInfoId change is expected and related to the PR.The artifact's buildInfoId changed in commit 6bf2a68 ("feat: do not use external input validator for programs"), which directly aligns with this PR's objective. This commit modified 40 files, including several related contracts (
Enclave.sol,IE3Program.sol,ExitQueueLib.sol,BondingRegistry.sol, etc.), which triggered a recompilation of dependent artifacts. SinceICiphernodeRegistry.solsource was not modified but its dependencies changed, the rebuilt artifact with the updated buildInfoId should be committed.Likely an incorrect or invalid review comment.
packages/enclave-contracts/contracts/test/MockComputeProvider.sol (1)
14-14: LGTM! Error naming follows Solidity conventions.Capitalizing the custom error name to
InvalidParamsaligns with Solidity naming conventions for custom errors (PascalCase).Also applies to: 21-21
examples/CRISP/server/.env.example (2)
15-15: LGTM! Comment accurately reflects Hardhat deployments.The comment update from "Anvil" to "Hardhat" correctly reflects the deployment context for the CRISP example.
18-18: Address consistency verified across CRISP deployment artifacts.The E3_PROGRAM_ADDRESS
0xc5a5C42992dECbae36851359345FE25997F5C42dis consistently used inenclave.config.yaml,deployed_contracts.json, and.env.example. No inconsistencies detected.examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts (1)
42-50: LGTM! Deployment updated to match new CRISPProgram constructor.The test correctly removes the inputValidatorFactory parameter from the CRISPProgram deployment, aligning with the architectural change to consolidate input validation within the program contract itself.
docs/pages/CRISP/introduction.mdx (1)
79-82: LGTM! Documentation clearly presents the validateInput interface.The updated documentation format with an explicit function signature makes the validateInput interface clearer and more accessible to developers. The validation responsibilities are well-articulated in the bullet points.
packages/enclave-contracts/ignition/modules/mockE3Program.ts (1)
11-11: LGTM! MockE3Program instantiation correctly updated.The removal of the mockInputValidator constructor argument aligns with the architectural change to consolidate validation within the program contract. The empty constructor arguments array correctly reflects the updated MockE3Program signature.
tests/integration/enclave.config.yaml (1)
6-6: Address verified as consistent with deployment artifacts.The e3_program address
0x322813Fd9A801c5507c9de605d63CEA4f2CE6c44matches the standard deployment artifacts and is documented as the default E3 program address for local integration testing. The address appears in deployed_contracts.json files across templates, packages, and examples, confirming consistency with actual test deployments.templates/default/enclave.config.yaml (1)
6-6: Verification complete: e3_program address is consistent with deployment artifacts.The address
0xc5a5C42992dECbae36851359345FE25997F5C42dinenclave.config.yamlmatches the deployment artifact indeployed_contracts.json(where it's labeled as "MyProgram"). The same address is also consistently used across related example configurations, confirming proper synchronization between configuration and deployment metadata.packages/enclave-contracts/test/Enclave.spec.ts (3)
306-306: LGTM! Simplified mock deployment.The removal of inputValidator from MockE3Program deployment aligns with the PR objective to consolidate input validation within the program contract itself.
528-552: LGTM! Test updated correctly.The test has been properly updated to no longer check for
e3.inputValidator, which is consistent with the removal of the external input validator pattern.
898-923: LGTM! Test assertions updated correctly.The test correctly validates E3 instantiation without checking for
inputValidator, reflecting the new architecture where validation is handled by the program contract.templates/default/deploy/default.ts (1)
39-49: LGTM! Deployment script updated correctly.The MyProgram deployment has been properly updated to use three constructor arguments (enclave, verifier, programId) instead of four, removing the inputValidator parameter. The stored constructorArgs also correctly reflect this change.
packages/enclave-contracts/deployed_contracts.json (1)
112-198: LGTM! Deployment records updated correctly.The deployment records have been properly updated to reflect the new architecture:
- MockE3Program replaces MockInputValidator
- Implementation addresses updated for re-deployed contracts
- Block numbers reflect the new deployments
packages/enclave-contracts/contracts/interfaces/IE3Program.sol (2)
40-48: LGTM! New validateInput function added.The new
validateInput()function enables programs to handle input validation internally, returning the decoded and validated application payload. This aligns with the PR objective to remove the external input validator dependency.
21-26: All E3Program implementations already comply with the new signature.Verification confirms all four E3Program implementations in the codebase already return
bytes32and match the new interface signature. No implementations retain the old tuple return type, and no updates are required.docs/pages/computation-flow.mdx (1)
60-68: LGTM! Documentation updated correctly.The documentation has been properly updated to reference
validateInputinstead ofvalidatefor the input validation phase, accurately reflecting the new program-based validation flow.packages/enclave-contracts/contracts/Enclave.sol (2)
275-293: LGTM! Simplified E3 program validation.The
validate()call has been correctly updated to receive onlyencryptionSchemeIdinstead of a tuple. The removal of inputValidator handling simplifies the request flow and aligns with the new architecture.
338-364: LGTM! Input validation delegated to program contract.The
publishInput()function now correctly callse3.e3Program.validateInput(msg.sender, data)instead of using a separate input validator contract. This consolidates validation logic within the program itself, as intended by the PR.templates/default/deployed_contracts.json (1)
117-124: LGTM! Template deployment records updated.The MyProgram deployment record has been properly updated with the new constructor arguments (enclave, verifier, programId), removing the inputValidator parameter.
packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts (1)
14-49: LGTM! Mock program deployment simplified.The deployment script has been correctly updated to:
- Remove
mockInputValidatorfrom the interface and parameters- Deploy MockE3Program without constructor arguments
- Store deployment args without constructorArgs
This aligns with the new architecture where validation is handled internally by the program.
cedoor
force-pushed
the
feat/no-input-validator-external
branch
from
a1e8f93 to
74de4da
Compare
oh thought I updated it. if we are not using it then we can remove it ig |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (2)
62-84: Wait for CRISP program deployment before enabling it on Enclave.After calling
crispFactory.deploy(...), the deployment transaction may still be pending. CallingenableE3Programon an address without deployed code will revert. This issue was flagged in previous reviews.Apply this diff to wait for deployment:
const crisp = await crispFactory.deploy( enclaveAddress, verifier, honkVerifierAddress, IMAGE_ID, ) + await crisp.waitForDeployment(); const crispAddress = await crisp.getAddress();
41-43: Wait for HonkVerifier deployment before using the address.The
honkVerifier.deploy()call may still be pending whengetAddress()is called. This could cause issues if the address is used before the contract is deployed.Apply this diff to ensure deployment completes:
const honkVerifierFactory = await ethers.getContractFactory("HonkVerifier"); const honkVerifier = await honkVerifierFactory.deploy(); + await honkVerifier.waitForDeployment(); const honkVerifierAddress = await honkVerifier.getAddress();examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol (1)
119-121: Existing issue: zero-address validation.This issue has already been flagged in a previous review.
🧹 Nitpick comments (7)
templates/default/contracts/MyProgram.sol (1)
70-84: validateInput() function provides a good template.The new
validateInputfunction provides a minimal but functional template with:
- Empty data validation
- Clear placeholder comments for custom logic
- Reference to the CRISP example for guidance
Note that the
senderparameter is currently unused in this template, which is acceptable since developers will add their own validation logic. Consider adding a comment explaining whensendermight be useful (e.g., checking allowlists, permissions, or sender-specific constraints).Consider adding a comment about the
senderparameter:/// @notice Validates input /// @param sender The account that is submitting the input. +/// @dev The sender parameter can be used for access control, allowlists, or sender-specific validation. /// @param data The input to be verified.examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (1)
27-28: Remove unuseduseMockInputValidatorflag.The
useMockInputValidatorflag is defined but the script no longer deploys a separate input validator contract (validation is now handled by the Program contract). The flag only controls which Program contract to deploy (MockCRISPProgram vs CRISPProgram), which is somewhat misleading.Consider renaming for clarity:
- const useMockInputValidator = - Boolean(process.env.USE_MOCK_INPUT_VALIDATOR) ?? false; + const useMockProgram = + Boolean(process.env.USE_MOCK_PROGRAM) ?? false;And update the conditional:
- if (useMockInputValidator) { + if (useMockProgram) { console.log("Using MockCRISPProgram");examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol (5)
82-84: Refactorrequirestatements to userevertwith custom errors.The
require(condition, CustomError())pattern is non-idiomatic and wasteful. Solidity custom errors should be used withifchecks andrevertfor proper gas efficiency and standard conventions.Apply this diff:
- require(address(_enclave) != address(0), EnclaveAddressZero()); - require(address(_verifier) != address(0), VerifierAddressZero()); - require(address(_honkVerifier) != address(0), InvalidHonkVerifier()); + if (address(_enclave) == address(0)) revert EnclaveAddressZero(); + if (address(_verifier) == address(0)) revert VerifierAddressZero(); + if (address(_honkVerifier) == address(0)) revert InvalidHonkVerifier();
111-115: Consider validating the imageId value.The function allows setting
imageIdtobytes32(0), which would break proof verification. Adding a validation check would prevent misconfiguration.Consider adding validation:
function setImageId(bytes32 _imageId) external onlyOwner { + if (_imageId == bytes32(0)) revert("Invalid imageId"); imageId = _imageId; }
137-138: Refactorrequiretorevertwith custom errors.Same pattern issue as in the constructor. Use idiomatic
ifchecks withrevertfor custom errors.Apply this diff:
- require(authorizedContracts[msg.sender] || msg.sender == owner(), CallerNotAuthorized()); - require(paramsHashes[e3Id] == bytes32(0), E3AlreadyInitialized()); + if (!authorizedContracts[msg.sender] && msg.sender != owner()) revert CallerNotAuthorized(); + if (paramsHashes[e3Id] != bytes32(0)) revert E3AlreadyInitialized();
144-150: Consider documenting the data format and naming unused parameter.The function decodes
datainto 4 fields but only returns the third element (vote) without validation of the other components. For clarity, consider:
- Naming the unused address parameter with a leading underscore (e.g.,
_sender)- Adding a comment documenting the expected data format:
(bytes proof, bytes32[] merkleProof, bytes vote, address voter)Example:
- function validateInput(address, bytes memory data) external returns (bytes memory input) { + /// @param _sender The sender address (unused in mock) + /// @param data Encoded as (bytes proof, bytes32[] merkleProof, bytes vote, address voter) + function validateInput(address _sender, bytes memory data) external returns (bytes memory input) { if (!isDataSet) revert RoundDataNotSet(); if (data.length == 0) revert EmptyInputData(); - (,, bytes memory vote,) = abi.decode(data, (bytes, bytes32[], bytes, address)); + // Extract only the vote data; proof validation is omitted in mock + (,, bytes memory vote,) = abi.decode(data, (bytes, bytes32[], bytes, address)); input = vote; }
200-200: Refactorrequiretorevertwith custom error.Same idiomatic pattern issue. Use
ifcheck withrevertfor custom errors.Apply this diff:
- require(paramsHashes[e3Id] != bytes32(0), E3DoesNotExist()); + if (paramsHashes[e3Id] == bytes32(0)) revert E3DoesNotExist();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (60)
README.md(1 hunks)crates/evm-helpers/src/contracts.rs(0 hunks)crates/evm-helpers/src/events.rs(0 hunks)crates/evm-helpers/tests/fixtures/fake_enclave.sol(0 hunks)crates/indexer/tests/fixtures/fake_enclave.sol(0 hunks)docs/pages/CRISP/introduction.mdx(1 hunks)docs/pages/CRISP/setup.mdx(0 hunks)docs/pages/architecture-overview.mdx(1 hunks)docs/pages/building-with-enclave.mdx(3 hunks)docs/pages/computation-flow.mdx(1 hunks)docs/pages/putting-it-together.mdx(1 hunks)docs/pages/write-e3-contract.mdx(3 hunks)examples/CRISP/Readme.md(0 hunks)examples/CRISP/enclave.config.yaml(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidator.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidatorFactory.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol(4 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol(16 hunks)examples/CRISP/packages/crisp-contracts/contracts/ImageID.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPInputValidator.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockRISC0Verifier.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/RiscZeroGroth16Verifier.sol(1 hunks)examples/CRISP/packages/crisp-contracts/deploy/crisp.ts(4 hunks)examples/CRISP/packages/crisp-contracts/deployed_contracts.json(1 hunks)examples/CRISP/packages/crisp-contracts/hardhat.config.ts(0 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(1 hunks)examples/CRISP/server/.env.example(1 hunks)examples/CRISP/server/src/server/indexer.rs(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(2 hunks)packages/enclave-contracts/contracts/interfaces/IE3.sol(0 hunks)packages/enclave-contracts/contracts/interfaces/IE3Program.sol(2 hunks)packages/enclave-contracts/contracts/interfaces/IInputValidator.sol(0 hunks)packages/enclave-contracts/contracts/lib/ExitQueueLib.sol(2 hunks)packages/enclave-contracts/contracts/registry/BondingRegistry.sol(2 hunks)packages/enclave-contracts/contracts/test/MockComputeProvider.sol(1 hunks)packages/enclave-contracts/contracts/test/MockE3Program.sol(1 hunks)packages/enclave-contracts/contracts/test/MockInputValidator.sol(0 hunks)packages/enclave-contracts/contracts/token/EnclaveToken.sol(2 hunks)packages/enclave-contracts/deployed_contracts.json(4 hunks)packages/enclave-contracts/ignition/modules/mockE3Program.ts(1 hunks)packages/enclave-contracts/ignition/modules/mockInputValidator.ts(0 hunks)packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts(0 hunks)packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts(2 hunks)packages/enclave-contracts/scripts/deployMocks.ts(0 hunks)packages/enclave-contracts/scripts/index.ts(0 hunks)packages/enclave-contracts/tasks/enclave.ts(2 hunks)packages/enclave-contracts/test/Enclave.spec.ts(3 hunks)packages/enclave-sdk/src/types.ts(0 hunks)templates/default/contracts/InputValidator.sol(0 hunks)templates/default/contracts/MyProgram.sol(2 hunks)templates/default/deploy/default.ts(1 hunks)templates/default/deployed_contracts.json(1 hunks)templates/default/enclave.config.yaml(1 hunks)templates/default/hardhat.config.ts(0 hunks)tests/integration/enclave.config.yaml(1 hunks)
💤 Files with no reviewable changes (21)
- packages/enclave-contracts/scripts/deployMocks.ts
- packages/enclave-sdk/src/types.ts
- examples/CRISP/packages/crisp-contracts/hardhat.config.ts
- packages/enclave-contracts/contracts/test/MockInputValidator.sol
- crates/indexer/tests/fixtures/fake_enclave.sol
- templates/default/contracts/InputValidator.sol
- examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol
- packages/enclave-contracts/ignition/modules/mockInputValidator.ts
- crates/evm-helpers/src/contracts.rs
- packages/enclave-contracts/scripts/index.ts
- packages/enclave-contracts/contracts/interfaces/IInputValidator.sol
- examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidatorFactory.sol
- crates/evm-helpers/tests/fixtures/fake_enclave.sol
- docs/pages/CRISP/setup.mdx
- examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPInputValidator.sol
- crates/evm-helpers/src/events.rs
- templates/default/hardhat.config.ts
- examples/CRISP/Readme.md
- packages/enclave-contracts/contracts/interfaces/IE3.sol
- examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidator.sol
- packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts
✅ Files skipped from review due to trivial changes (3)
- examples/CRISP/packages/crisp-contracts/contracts/Mocks/RiscZeroGroth16Verifier.sol
- examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol
- examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockRISC0Verifier.sol
🚧 Files skipped from review as they are similar to previous changes (16)
- examples/CRISP/server/src/server/indexer.rs
- README.md
- templates/default/deploy/default.ts
- packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json
- packages/enclave-contracts/test/Enclave.spec.ts
- packages/enclave-contracts/tasks/enclave.ts
- packages/enclave-contracts/contracts/registry/BondingRegistry.sol
- packages/enclave-contracts/contracts/lib/ExitQueueLib.sol
- tests/integration/enclave.config.yaml
- packages/enclave-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.json
- examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts
- packages/enclave-contracts/ignition/modules/mockE3Program.ts
- docs/pages/computation-flow.mdx
- packages/enclave-contracts/deployed_contracts.json
- packages/enclave-contracts/contracts/Enclave.sol
- templates/default/enclave.config.yaml
🧰 Additional context used
🧠 Learnings (6)
📓 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.693Z
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.693Z
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.693Z
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:
docs/pages/CRISP/introduction.mdxexamples/CRISP/enclave.config.yamlexamples/CRISP/packages/crisp-contracts/deploy/crisp.tsdocs/pages/architecture-overview.mdxdocs/pages/putting-it-together.mdxtemplates/default/contracts/MyProgram.solexamples/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.soldocs/pages/building-with-enclave.mdxdocs/pages/write-e3-contract.mdxexamples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.solexamples/CRISP/server/.env.example
📚 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.yamltemplates/default/deployed_contracts.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json
📚 Learning: 2025-11-05T14:12:57.814Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 963
File: examples/CRISP/client/package.json:25-25
Timestamp: 2025-11-05T14:12:57.814Z
Learning: In the Enclave/CRISP codebase, `enclave-e3/sdk` and `crisp-e3/sdk` are different packages: `enclave-e3/sdk` is the general Enclave SDK, while `crisp-e3/sdk` is the CRISP-specific SDK. The CRISP client (`examples/CRISP/client`) intentionally depends on `enclave-e3/sdk`, not `crisp-e3/sdk`.
Applied to files:
examples/CRISP/packages/crisp-contracts/deploy/crisp.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/deploy/crisp.tspackages/enclave-contracts/scripts/deployAndSave/mockProgram.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:
examples/CRISP/server/.env.example
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (1)
packages/enclave-contracts/scripts/utils.ts (2)
storeDeploymentArgs(35-67)readDeploymentArgs(75-89)
🪛 dotenv-linter (4.0.0)
examples/CRISP/server/.env.example
[warning] 16-16: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 17-17: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 17-17: [UnorderedKey] The CIPHERNODE_REGISTRY_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
[warning] 18-18: [UnorderedKey] The E3_PROGRAM_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
🪛 LanguageTool
docs/pages/write-e3-contract.mdx
[style] ~67-~67: Consider a more concise word here.
Context: ...PRisc0.sol#L45). ### Input Validation In order to ensure correct computation, we should b...
(IN_ORDER_TO_PREMIUM)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build_sdk
- GitHub Check: build_e3_support_risc0
- GitHub Check: rust_integration
- GitHub Check: rust_unit
- GitHub Check: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: test_net
- GitHub Check: test_contracts
- GitHub Check: Build & Push Image
🔇 Additional comments (34)
examples/CRISP/packages/crisp-contracts/contracts/ImageID.sol (1)
22-22: LGTM—formatting-only change.The reformatting from multiline to single-line is purely cosmetic with no functional impact. Since the file is auto-generated, this likely reflects an update to the generation script.
packages/enclave-contracts/contracts/token/EnclaveToken.sol (2)
152-154: Good practice: braces improve clarity and safety.Adding braces around the single-line conditional improves consistency and reduces the risk of introducing bugs during future modifications.
236-238: Good practice: braces improve clarity and safety.Adding braces around the single-line conditional improves consistency and reduces the risk of introducing bugs during future modifications.
packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts (3)
14-16: LGTM! Clean removal of input validator dependency.The interface correctly reflects the removal of the external input validator requirement, aligning with the PR objectives to move validation directly into program contracts.
28-28: LGTM! Simplified deployment.The no-argument deployment correctly reflects that MockE3Program no longer requires an input validator in its constructor, consistent with the architectural change.
35-42: LGTM! Appropriate simplification of deployment metadata.Storing only
blockNumberandaddressis correct since MockE3Program no longer has constructor arguments. The removal of pre-deployment reuse logic simplifies the deployment flow and aligns with the cleaner architecture.docs/pages/putting-it-together.mdx (1)
106-131: Documentation accurately reflects the architectural changes.The constructor example correctly shows the removal of the InputValidator parameter, aligning with the PR's objective to move input validation into the E3 Program contract itself.
packages/enclave-contracts/contracts/test/MockComputeProvider.sol (1)
14-21: LGTM: Improved naming convention.The error has been renamed from
invalidParams()toInvalidParams()to follow Solidity naming conventions where custom errors use PascalCase. This is a good improvement with no functional impact.templates/default/deployed_contracts.json (1)
70-124: Deployment addresses updated correctly.The deployment manifest has been updated to reflect the redeployed contracts after removing the InputValidator pattern. The MyProgram constructorArgs correctly reference the updated Enclave and verifier addresses without any inputValidator references.
templates/default/contracts/MyProgram.sol (2)
32-49: Constructor correctly updated to remove InputValidator.The constructor signature has been properly updated to remove the
_inputValidatorparameter, and the newEmptyInputDataerror has been added to support thevalidateInputfunction. The changes are clean and consistent with the PR objectives.
54-68: validate() function signature correctly updated.The function now returns only
bytes32instead of(bytes32, IInputValidator), correctly reflecting that input validation is no longer delegated to a separate contract. The implementation is correct.examples/CRISP/packages/crisp-contracts/deployed_contracts.json (1)
61-68: CRISP deployment manifest correctly updated.The CRISPProgram constructorArgs have been properly restructured to include
verifierAddressandhonkVerifierAddresswhile removing theinputValidatorreferences. The changes are consistent across both sepolia and localhost deployments.Also applies to: 182-189
examples/CRISP/server/.env.example (1)
15-18: Environment configuration correctly updated.The E3_PROGRAM_ADDRESS has been updated to
0xc5a5C42992dECbae36851359345FE25997F5C42d, which correctly matches the CRISPProgram address in the deployed_contracts.json file. The comment has also been updated to accurately reflect "Hardhat Deployments."examples/CRISP/enclave.config.yaml (1)
6-6: Configuration address correctly updated.The e3_program address has been updated to match the new CRISPProgram deployment address, consistent with deployed_contracts.json and .env.example.
docs/pages/CRISP/introduction.mdx (1)
79-83: Documentation accurately updated.The documentation has been updated to reflect that
validateInput()is now a function in the CRISPProgram contract rather than a separate CRISPInputValidator contract. The listed responsibilities (format validation, eligibility proof verification, uniqueness checks) accurately describe what the function does.docs/pages/architecture-overview.mdx (1)
43-45: LGTM! Documentation accurately reflects the new architecture.The updated documentation correctly describes the E3 Program's expanded responsibility for input validation, including ZKP verification and input sanitization. This aligns well with the PR's objective to move validation logic into the program contract.
packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json (1)
965-965: LGTM! Artifact correctly reflects the interface changes.The buildInfoId update and removal of inputValidator from the E3 struct tuples throughout the ABI are consistent with the PR's objective to eliminate the external input validator.
packages/enclave-contracts/contracts/interfaces/IE3Program.sol (2)
21-26: LGTM! The validate signature update is correct.The removal of
IInputValidatorfrom the return type aligns with the PR's objective to eliminate the external input validator contract. Programs now return only the encryption scheme ID.
40-48: LGTM! The new validateInput function API is well-designed.The function signature correctly captures the essential parameters (sender address and input data) and returns the validated input payload. The documentation clearly explains the function's purpose and when it's called by the Enclave contract.
docs/pages/building-with-enclave.mdx (1)
120-127: LGTM! Documentation accurately describes the new input validation flow.The updated example correctly shows that the Enclave contract now calls
validateInput()on the Program contract. The function signature matches the IE3Program interface.docs/pages/write-e3-contract.mdx (2)
26-33: LGTM! The validate signature update is correctly documented.The documentation accurately reflects that
validatenow returns only theencryptionSchemeId, consistent with the interface changes.
66-87: LGTM! The input validation documentation is clear and accurate.The rewritten section correctly describes the new approach where input validation is implemented directly in the E3 Program contract via
validateInput. The example code demonstrates the expected function signature and validation flow.packages/enclave-contracts/contracts/test/MockE3Program.sol (3)
8-20: LGTM! Mock contract correctly updated to match the new interface.The removal of
IInputValidatordependencies and the simplified constructor align with the IE3Program interface changes. The updated error definitions are appropriate for the test mock.
22-36: LGTM! The validate function correctly implements the updated signature.The function now returns only
bytes32(encryption scheme ID) instead of a tuple, consistent with the IE3Program interface changes.
38-47: LGTM! The validateInput implementation is appropriate for testing.The mock validation logic (rejecting data.length == 3 or zero sender) provides useful test coverage for error conditions while remaining simple for a test mock.
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol (6)
16-26: LGTM! The RoundData structure is well-defined.The struct appropriately captures the essential parameters for a voting round: governance token, balance threshold, snapshot block, and census merkle root. Clear documentation enhances maintainability.
38-44: LGTM! State variables appropriately support the new validation flow.The
roundDataandvoteSlotsmapping enable the program to manage round-specific configuration and per-address vote storage, consistent with the inline validation approach.
79-91: LGTM! Constructor correctly updated for the new architecture.The removal of the input validator parameter and the streamlined initialization align with the PR's objective to eliminate the external input validator contract.
95-109: LGTM! The setRoundData function provides proper idempotency control.The function correctly enforces that round data can only be set once via the
isDataSetflag, preventing accidental or malicious reconfiguration.
134-143: LGTM! The validate function correctly implements the updated signature.The function now returns only the encryption scheme ID, consistent with the IE3Program interface changes. Authorization and initialization checks remain intact.
145-172: LGTM! The validateInput implementation is secure and well-structured.The function correctly:
- Enforces authorization (addressing the previous review concern)
- Validates input data length
- Decodes and verifies the Noir proof cryptographically
- Stores the vote in the appropriate slot
- Returns the validated input for merkle tree inclusion
The authorization check at line 147 ensures only the Enclave (or owner) can call this function, preventing direct manipulation of vote storage. The cryptographic proof verification provides the primary security boundary.
Based on learnings
examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol (3)
93-109: LGTM! Proper idempotency guard.The function correctly prevents re-initialization with the
isDataSetcheck and sets the flag before mutating state. Access control is properly enforced.
212-222: LGTM! Correct length-prefix encoding.The function correctly implements 4-byte aligned encoding with a length prefix, matching the RISC Zero journal format. The math checks out: each hash occupies 132 bytes (1 length byte + 3 padding + 32 hash bytes at 4-byte intervals), and 3 hashes = 396 bytes as allocated in the
verifyfunction.
152-188: Add length validation to prevent potential underflow in decodeTally.The function calculates
START_INDEX_Y = halfD - HALF_LARGEST_MINIMUM_DEGREEandSTART_INDEX_N = tally.length - HALF_LARGEST_MINIMUM_DEGREEwithout ensuringtally.length >= 2 * HALF_LARGEST_MINIMUM_DEGREE(56 elements). If the plaintext output array is shorter than expected, these subtractions will underflow in Solidity's uint256 arithmetic, wrapping to very large numbers.Add a require statement at the start of the loop section:
require(tally.length >= 2 * HALF_LARGEST_MINIMUM_DEGREE, "Invalid tally length");This applies to both
examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.solandexamples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol(lines 187–189 and 165–167 respectively).
ctrlc03
force-pushed
the
feat/no-input-validator-external
branch
from
74de4da to
73f72e4
Compare
ctrlc03
force-pushed
the
feat/no-input-validator-external
branch
from
73f72e4 to
27b118a
Compare
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 (2)
packages/enclave-contracts/contracts/interfaces/IE3Program.sol (1)
14-26: Update the function documentation to reflect the signature change.The comment on line 14 states "return encryption scheme and input validator", but the function now only returns the encryption scheme ID. The validate() function no longer returns an IInputValidator instance.
Apply this diff to update the comment:
- /// @notice Validate E3 computation parameters and return encryption scheme and input validator + /// @notice Validate E3 computation parameters and return encryption scheme /// @dev This function is called by the Enclave contract during E3 request to configure the computationtemplates/default/deployed_contracts.json (1)
111-122: Fix duplicate key declarations in JSON.The JSON object has duplicate declarations for both
MockRISC0VerifierandImageID. In JSON, when a key is defined multiple times, only the last definition takes effect, making the earlier definitions at lines 111-116 unreachable.Review which addresses are correct for each contract and remove the duplicates. If these represent different contracts or deployment stages, consider using distinct keys or restructuring the data.
- "MockRISC0Verifier": { - "address": "0x7a2088a1bFc9d81c55368AE168C2C02570cB814F" - }, - "ImageID": { - "address": "0x09635F643e140090A9A8Dcd712eD6285858ceBef" - }, "MockRISC0Verifier": { "address": "0x09635F643e140090A9A8Dcd712eD6285858ceBef" }, "ImageID": { "address": "0xc5a5C42992dECbae36851359345FE25997F5C42d" },Note: Verify these are the correct addresses before applying this change.
♻️ Duplicate comments (4)
packages/enclave-contracts/contracts/lib/ExitQueueLib.sol (1)
145-148: Inconsistent brace formatting between adjacent equivalent statements.Line 145 lacks braces while lines 146-148 have braces, yet both perform equivalent operations (updating amounts in the same tranche). For consistency, either add braces to line 145 or remove them from lines 146-148.
Apply this diff to add braces to line 145 for consistency:
- if (ticketAmount != 0) lastTranche.ticketAmount += ticketAmount; + if (ticketAmount != 0) { + lastTranche.ticketAmount += ticketAmount; + } if (licenseAmount != 0) { lastTranche.licenseAmount += licenseAmount; }examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (1)
40-68: MissingwaitForDeployment()calls before enabling the program.After deploying both
honkVerifier(line 41) andcrisp(lines 61-66), you must await deployment confirmation before callingenableE3Program(line 80). WithoutwaitForDeployment(), the addresses may not have on-chain code yet, causing the enable transaction to revert.Apply this diff:
const honkVerifierFactory = await ethers.getContractFactory("HonkVerifier"); const honkVerifier = await honkVerifierFactory.deploy(); + await honkVerifier.waitForDeployment(); const honkVerifierAddress = await honkVerifier.getAddress();const crisp = await crispFactory.deploy( enclaveAddress, verifier, honkVerifierAddress, IMAGE_ID, ) + await crisp.waitForDeployment(); const crispAddress = await crisp.getAddress();examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol (2)
119-121: Guard against zero verifier address.If
_verifieris set to the zero address, subsequentverifier.verifycalls will revert, breaking proof verification. Add validation using the existingVerifierAddressZeroerror.Apply this diff:
function setVerifier(IRiscZeroVerifier _verifier) external onlyOwner { + if (address(_verifier) == address(0)) revert VerifierAddressZero(); verifier = _verifier; }
144-150: Missing round data initialization check.The function doesn't verify that
isDataSetis true before processing input. This allows validation to succeed even when round parameters haven't been configured viasetRoundData, potentially accepting votes for an uninitialized round.Add a guard at the start:
function validateInput(address, bytes memory data) external returns (bytes memory input) { + if (!isDataSet) revert RoundDataNotSet(); if (data.length == 0) revert EmptyInputData();
🧹 Nitpick comments (1)
examples/CRISP/packages/crisp-contracts/deployed_contracts.json (1)
124-126: Consider removing orphaned CRISPInputValidator entry.The
CRISPInputValidatorcontract address is no longer referenced anywhere in the deployment config (it's not a dependency of any constructor arguments). Per the PR objectives, input validation is now handled by the program directly. This entry appears to be leftover configuration that could cause confusion during future deployments or maintenance.Unless retained for historical reference, consider removing this entry to keep the config clean and aligned with the new architecture. Based on learnings
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (60)
README.md(1 hunks)crates/evm-helpers/src/contracts.rs(0 hunks)crates/evm-helpers/src/events.rs(0 hunks)crates/evm-helpers/tests/fixtures/fake_enclave.sol(0 hunks)crates/indexer/tests/fixtures/fake_enclave.sol(0 hunks)docs/pages/CRISP/introduction.mdx(1 hunks)docs/pages/CRISP/setup.mdx(0 hunks)docs/pages/architecture-overview.mdx(1 hunks)docs/pages/building-with-enclave.mdx(3 hunks)docs/pages/computation-flow.mdx(1 hunks)docs/pages/putting-it-together.mdx(1 hunks)docs/pages/write-e3-contract.mdx(3 hunks)examples/CRISP/Readme.md(0 hunks)examples/CRISP/client/.env.example(0 hunks)examples/CRISP/client/src/config/Enclave.abi.ts(0 hunks)examples/CRISP/enclave.config.yaml(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidator.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidatorFactory.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol(4 hunks)examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/ImageID.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPInputValidator.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol(0 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockRISC0Verifier.sol(1 hunks)examples/CRISP/packages/crisp-contracts/contracts/Mocks/RiscZeroGroth16Verifier.sol(1 hunks)examples/CRISP/packages/crisp-contracts/deploy/crisp.ts(4 hunks)examples/CRISP/packages/crisp-contracts/deployed_contracts.json(1 hunks)examples/CRISP/packages/crisp-contracts/hardhat.config.ts(0 hunks)examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts(1 hunks)examples/CRISP/server/.env.example(1 hunks)examples/CRISP/server/src/server/indexer.rs(1 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json(1 hunks)packages/enclave-contracts/contracts/Enclave.sol(2 hunks)packages/enclave-contracts/contracts/interfaces/IE3.sol(0 hunks)packages/enclave-contracts/contracts/interfaces/IE3Program.sol(2 hunks)packages/enclave-contracts/contracts/interfaces/IInputValidator.sol(0 hunks)packages/enclave-contracts/contracts/lib/ExitQueueLib.sol(2 hunks)packages/enclave-contracts/contracts/registry/BondingRegistry.sol(2 hunks)packages/enclave-contracts/contracts/test/MockComputeProvider.sol(1 hunks)packages/enclave-contracts/contracts/test/MockE3Program.sol(1 hunks)packages/enclave-contracts/contracts/test/MockInputValidator.sol(0 hunks)packages/enclave-contracts/contracts/token/EnclaveToken.sol(2 hunks)packages/enclave-contracts/deployed_contracts.json(1 hunks)packages/enclave-contracts/ignition/modules/mockE3Program.ts(1 hunks)packages/enclave-contracts/ignition/modules/mockInputValidator.ts(0 hunks)packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts(0 hunks)packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts(2 hunks)packages/enclave-contracts/scripts/deployMocks.ts(0 hunks)packages/enclave-contracts/scripts/index.ts(0 hunks)packages/enclave-contracts/tasks/enclave.ts(2 hunks)packages/enclave-contracts/test/Enclave.spec.ts(3 hunks)packages/enclave-sdk/src/types.ts(0 hunks)templates/default/contracts/InputValidator.sol(0 hunks)templates/default/contracts/MyProgram.sol(2 hunks)templates/default/deploy/default.ts(1 hunks)templates/default/deployed_contracts.json(2 hunks)templates/default/enclave.config.yaml(1 hunks)templates/default/hardhat.config.ts(0 hunks)tests/integration/enclave.config.yaml(1 hunks)
💤 Files with no reviewable changes (23)
- docs/pages/CRISP/setup.mdx
- templates/default/contracts/InputValidator.sol
- packages/enclave-contracts/scripts/index.ts
- examples/CRISP/client/src/config/Enclave.abi.ts
- crates/evm-helpers/tests/fixtures/fake_enclave.sol
- packages/enclave-sdk/src/types.ts
- packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts
- examples/CRISP/client/.env.example
- examples/CRISP/packages/crisp-contracts/hardhat.config.ts
- examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPInputValidator.sol
- packages/enclave-contracts/scripts/deployMocks.ts
- packages/enclave-contracts/contracts/interfaces/IE3.sol
- crates/evm-helpers/src/events.rs
- examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidatorFactory.sol
- crates/indexer/tests/fixtures/fake_enclave.sol
- examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol
- packages/enclave-contracts/ignition/modules/mockInputValidator.ts
- templates/default/hardhat.config.ts
- packages/enclave-contracts/contracts/test/MockInputValidator.sol
- examples/CRISP/Readme.md
- packages/enclave-contracts/contracts/interfaces/IInputValidator.sol
- examples/CRISP/packages/crisp-contracts/contracts/CRISPInputValidator.sol
- crates/evm-helpers/src/contracts.rs
✅ Files skipped from review due to trivial changes (1)
- examples/CRISP/packages/crisp-contracts/contracts/CRISPVerifier.sol
🚧 Files skipped from review as they are similar to previous changes (18)
- examples/CRISP/enclave.config.yaml
- examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockRISC0Verifier.sol
- packages/enclave-contracts/test/Enclave.spec.ts
- packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts
- packages/enclave-contracts/tasks/enclave.ts
- tests/integration/enclave.config.yaml
- packages/enclave-contracts/contracts/token/EnclaveToken.sol
- packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json
- packages/enclave-contracts/contracts/Enclave.sol
- packages/enclave-contracts/contracts/test/MockComputeProvider.sol
- packages/enclave-contracts/contracts/registry/BondingRegistry.sol
- README.md
- examples/CRISP/server/src/server/indexer.rs
- docs/pages/architecture-overview.mdx
- examples/CRISP/packages/crisp-contracts/contracts/Mocks/RiscZeroGroth16Verifier.sol
- examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol
- examples/CRISP/packages/crisp-contracts/tests/crisp.contracts.test.ts
- docs/pages/computation-flow.mdx
🧰 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-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/deploy/default.tspackages/enclave-contracts/contracts/interfaces/IE3Program.soldocs/pages/putting-it-together.mdxdocs/pages/CRISP/introduction.mdxexamples/CRISP/packages/crisp-contracts/deploy/crisp.tstemplates/default/contracts/MyProgram.solpackages/enclave-contracts/deployed_contracts.jsonpackages/enclave-contracts/contracts/test/MockE3Program.soldocs/pages/building-with-enclave.mdxexamples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.solexamples/CRISP/packages/crisp-contracts/contracts/ImageID.solexamples/CRISP/packages/crisp-contracts/deployed_contracts.jsontemplates/default/deployed_contracts.jsondocs/pages/write-e3-contract.mdx
📚 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.exampleexamples/CRISP/packages/crisp-contracts/deploy/crisp.tstemplates/default/enclave.config.yaml
📚 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/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:
packages/enclave-contracts/deployed_contracts.jsontemplates/default/enclave.config.yamldocs/pages/building-with-enclave.mdx
🧬 Code graph analysis (1)
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (1)
packages/enclave-contracts/scripts/utils.ts (2)
storeDeploymentArgs(56-88)readDeploymentArgs(96-110)
🪛 Biome (2.1.2)
templates/default/deployed_contracts.json
[error] 111-111: The key MockRISC0Verifier was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
[error] 114-114: The key ImageID was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🪛 dotenv-linter (4.0.0)
examples/CRISP/server/.env.example
[warning] 16-16: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 17-17: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 17-17: [UnorderedKey] The CIPHERNODE_REGISTRY_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
[warning] 18-18: [UnorderedKey] The E3_PROGRAM_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
🪛 LanguageTool
docs/pages/write-e3-contract.mdx
[style] ~67-~67: Consider a more concise word here.
Context: ...PRisc0.sol#L45). ### Input Validation In order to ensure correct computation, we should b...
(IN_ORDER_TO_PREMIUM)
⏰ 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: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: build_sdk
- GitHub Check: build_e3_support_dev
- GitHub Check: test_net
- GitHub Check: test_contracts
- GitHub Check: rust_integration
- GitHub Check: rust_unit
- GitHub Check: Build & Push Image
- GitHub Check: Build & Push Image
🔇 Additional comments (26)
examples/CRISP/packages/crisp-contracts/deployed_contracts.json (1)
115-122: CRISPProgram deployment changes align with PR objectives.The constructorArgs update correctly removes the
inputValidatorreference and updates verifier addresses. The program will now handle input validation directly.packages/enclave-contracts/deployed_contracts.json (1)
55-58: Verify updated MockE3Program deployment details.The
MockE3Programentry shows a redeployed contract with a new address and earlier blockNumber. The removal ofMockInputValidatoris correct and aligns with the PR objective. However, the blockNumber (9479405) appears earlier than other recent deployments like PoseidonT3 (9615389), which warrants verification.Please confirm:
- The new address
0x5a196784e60A6A18b86Af7a9e564A969F6d2bC76is the correct sepolia deployment- The blockNumber
9479405is accurate (whether this reflects a different deployment sequence or if a more recent deployment exists)- Whether
constructorArgsshould be added forMockE3Programif the contract now requires initialization parameters due to the consolidation of validation logicexamples/CRISP/packages/crisp-contracts/contracts/ImageID.sol (1)
22-22: LGTM! Cosmetic formatting change.The reformatting of
PROGRAM_IDfrom multiline to single-line is a cosmetic change with no functional impact. Since this is an automatically generated file, the formatting update is appropriate.packages/enclave-contracts/contracts/lib/ExitQueueLib.sol (1)
379-391: Consistent brace formatting improves readability.All four if statements in the increase/decrease branches now consistently use braces, making the code more uniform and maintainable.
packages/enclave-contracts/ignition/modules/mockE3Program.ts (1)
11-11: Verification confirmed: MockE3Program constructor accepts zero parameters.The MockE3Program contract has no explicit constructor defined, meaning it uses Solidity's implicit default constructor that accepts zero parameters. The deployment change to use an empty array
[]is correct and compatible.templates/default/deploy/default.ts (1)
39-49: LGTM! InputValidator correctly removed from deployment.The deployment script has been properly updated to remove the InputValidator contract deployment and its reference from MyProgram's constructor arguments. The constructor now correctly receives only the enclave, verifier, and programId parameters, aligning with the updated IE3Program interface.
docs/pages/putting-it-together.mdx (1)
106-131: LGTM! Documentation correctly updated to reflect new validation pattern.The example code has been properly updated to show initialization without an InputValidator parameter, and the imports have been cleaned up accordingly. This aligns with the shift to Program-based input validation.
docs/pages/CRISP/introduction.mdx (1)
79-82: LGTM! Clear documentation of the new validation function.The documentation clearly describes the
validateInput()function and its responsibilities, accurately reflecting the architectural change from a separate InputValidator contract to inline validation within the Program contract.templates/default/enclave.config.yaml (1)
6-6: LGTM! E3 program address correctly updated.The address has been updated to match the newly deployed MyProgram contract (verified against
deployed_contracts.json), reflecting the changes made to remove InputValidator dependencies.packages/enclave-contracts/contracts/interfaces/IE3Program.sol (1)
40-48: LGTM! Well-defined validateInput() function.The new
validateInput()function is clearly documented and has an appropriate signature for handling input validation directly in the Program contract. The flexible return type allows for various input formats.docs/pages/building-with-enclave.mdx (1)
59-59: LGTM! Documentation narrative correctly updated.The documentation accurately describes the new architecture where the Enclave contract calls
validateInput()directly on the E3 Program contract instead of using a separate InputValidator contract.Also applies to: 120-121, 221-223
templates/default/deployed_contracts.json (2)
127-133: LGTM! MyProgram entry correctly updated (pending duplicate key fix).The MyProgram deployment entry has been properly updated to remove the inputValidator field from constructorArgs and update the address and verifier references. However, note that the verifier address (0x7a2088a1bFc9d81c55368AE168C2C02570cB814F) references the first (ignored) MockRISC0Verifier declaration, so this will need verification once the duplicate key issue is resolved.
107-110: LGTM! MockE3Program correctly added to replace MockInputValidator.The MockE3Program entry has been added appropriately to support the new validation architecture, replacing the removed MockInputValidator.
docs/pages/write-e3-contract.mdx (2)
66-76: Documentation accurately reflects the architectural change.The updated section correctly describes the new inline validation approach where input validation is implemented directly in the E3 Program contract rather than in a separate Input Validator contract.
79-88: Clear example for inline input validation.The example correctly demonstrates the
validateInputfunction signature and provides helpful placeholder comments for developers to implement their own validation logic.templates/default/contracts/MyProgram.sol (3)
38-49: Constructor correctly simplified.The constructor properly validates the verifier address and initializes the contract without the now-removed
inputValidatorparameter. The authorization of the enclave address is correctly maintained.
54-68: Validate function correctly updated.The function signature now returns only the encryption scheme ID as per the new IE3Program interface. Access control and initialization checks are properly maintained.
70-84: Good template for input validation.The
validateInputfunction provides a solid starting point with:
- Empty data validation
- Clear placeholder comments for custom logic
- Reference to the CRISP example implementation
This gives developers the structure they need to implement their own validation.
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (1)
52-59: Factory selection logic is sound.The conditional selection between
MockCRISPProgramandCRISPProgrambased on theuseMockInputValidatorflag provides flexibility for testing and production deployments.packages/enclave-contracts/contracts/test/MockE3Program.sol (2)
20-34: Mock validate function correctly implements the new interface.The function properly validates parameters, prevents re-initialization, and returns only the encryption scheme ID as required by the updated IE3Program interface.
36-45: Mock validateInput provides appropriate test conditions.The mock implementation includes simple validation logic (checking data length and sender address) that allows testing both success and failure paths in the test suite.
examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockCRISPProgram.sol (5)
79-91: Constructor has proper address validation.The constructor correctly validates that
_enclave,_verifier, and_honkVerifierare non-zero addresses before initialization, preventing misconfiguration.
95-109: Round data initialization is idempotent and secure.The function correctly prevents re-initialization with the
RoundDataAlreadySetcheck and properly restricts access to the owner.
133-142: Validate function correctly implements the updated interface.The function properly enforces access control, prevents re-initialization, and returns only the encryption scheme ID as per the new IE3Program interface.
156-188: Tally decoding logic implements the voting scheme correctly.The function correctly:
- Fetches the plaintext output from the enclave
- Isolates the vote coefficients using the
HALF_LARGEST_MINIMUM_DEGREEconstant- Converts bit-weighted values back to integers for both yes and no votes
The bit-weighting approach with decreasing powers of 2 is appropriate for the polynomial-based vote storage scheme.
194-210: Verification logic is correct.The function properly:
- Validates the E3 exists
- Constructs the journal with the required components (ciphertext output, params hash, input root)
- Verifies the proof using the configured RISC Zero verifier
ctrlc03
force-pushed
the
feat/no-input-validator-external
branch
from
6090eb7 to
38f269a
Compare
2 participants
Talk directly to the program contract for input validation. app developers may still want to use a separate contract
fix #997
skipping line limit cause there's a bunch of deleted files
Summary by CodeRabbit
Refactor
Documentation