chore: update risc0 to v3#815
Conversation
* chore: update zkvm to v3.0.0 * chore: update deployment script (cherry picked from commit 2d9133e)
* chore: update risc0 version * chore: remove risc0 crates from CRISP (cherry picked from commit c94f5f3)
(cherry picked from commit c2d7827)
* fix: risc0 dev mode env var * chore: lint * chore: add dev comment (cherry picked from commit 7e09bad)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughSolidity Enclave interface and Rust bindings updated (bytes memory→calldata; getRoot→getInputRoot). RISC0 toolchain/dependencies bumped to 3.x. CRISP: ImageID constant renamed/changed, factory now accepts an implementation address, mock validator added, deploy flow/logging adjusted, dev-mode host behavior tightened to "1". Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Deployer
participant DS as Deploy.s.sol
participant V as Validator (Mock/Real)
participant F as CRISPInputValidatorFactory
participant P as CRISPProgram
Deployer->>DS: run deployment
DS->>DS: check USE_MOCK_INPUT_VALIDATOR
alt USE_MOCK_INPUT_VALIDATOR == true
DS->>V: deploy MockCRISPInputValidator
else
DS->>V: deploy CRISPInputValidator
end
DS->>F: deploy Factory(address V)
DS->>P: deploy CRISPProgram(ImageID.PROGRAM_ID)
DS-->>Deployer: log deployed addresses (consolidated)
sequenceDiagram
autonumber
participant App as Caller
participant H as support::host (lib.rs)
App->>H: generate_seal(...)
alt RISC0_DEV_MODE == "1"
H-->>App: return empty seal (dev-mode)
else
H->>H: compute Groth16 proof
H-->>App: return real seal
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧬 Code graph analysis (2)examples/CRISP/server/src/cli/commands.rs (1)
crates/evm-helpers/src/contracts.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
🔇 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 |
(cherry picked from commit 2861d9b)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
crates/support/host/src/lib.rs (1)
62-65: Consider using a structured logging framework.The
println!statements for dev mode tracking go directly to stdout. For better observability in production, consider using a logging framework (e.g.,tracing,log) with appropriate log levels.examples/CRISP/contracts/Mocks/MockCRISPInputValidator.sol (1)
38-48: Consider validating all decoded components.The mock validator only extracts the
votecomponent from the decoded data tuple(bytes, bytes, bytes32[], bytes)and ignores the other three components. While this may be intentional for a mock, consider whether this adequately mimics the real validator's behavior for testing purposes.Additionally, the declared but unused state variables
policyandnoirVerifier(initialized in_initialize) suggest incomplete validation logic.Apply this diff if you want to add validation using the initialized components:
function validate(address sender, bytes memory data) external returns (bytes memory input) { if (data.length == 0) revert EmptyInputData(); - (,,, bytes memory vote) = abi.decode(data, (bytes, bytes, bytes32[], bytes)); + (bytes memory proof, bytes memory publicInputs, bytes32[] memory merkleProof, bytes memory vote) = abi.decode(data, (bytes, bytes, bytes32[], bytes)); + + // Validate proof (example - adjust based on actual requirements) + if (!noirVerifier.verify(proof, publicInputs)) { + revert InvalidNoirProof(); + } + + // Validate policy (example - adjust based on actual requirements) + try policy.check(sender, vote) { + // Policy check passed + } catch (bytes memory reason) { + revert InvalidInputData(reason); + } input = vote; }examples/CRISP/contracts/CRISPInputValidatorFactory.sol (1)
14-15: Update the comment to reflect the new constructor signature.The comment on line 14 states "Initializes the factory with the CRISPInputValidator implementation" but doesn't mention that the implementation must now be provided externally rather than being created internally.
Consider updating the comment to clarify the new behavior:
- /// @notice Initializes the factory with the CRISPInputValidator implementation. + /// @notice Initializes the factory with an external CRISPInputValidator implementation. + /// @param inputValidator Address of the CRISPInputValidator implementation contract. constructor(address inputValidator) Factory(inputValidator) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/support/Cargo.lockis excluded by!**/*.lockcrates/support/methods/guest/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
crates/evm-helpers/src/contracts.rs(3 hunks)crates/support/Cargo.toml(1 hunks)crates/support/Dockerfile(1 hunks)crates/support/contracts/ImageID.sol(1 hunks)crates/support/host/src/lib.rs(1 hunks)crates/support/methods/guest/Cargo.toml(1 hunks)examples/CRISP/.gitignore(1 hunks)examples/CRISP/Cargo.toml(0 hunks)examples/CRISP/client/.env.example(1 hunks)examples/CRISP/contracts/CRISPInputValidatorFactory.sol(1 hunks)examples/CRISP/contracts/ImageID.sol(1 hunks)examples/CRISP/contracts/Mocks/MockCRISPInputValidator.sol(1 hunks)examples/CRISP/deploy/Deploy.s.sol(3 hunks)examples/CRISP/enclave.config.yaml(1 hunks)examples/CRISP/lib/risc0-ethereum(1 hunks)examples/CRISP/server/.env.example(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/artifacts/contracts/registry/NaiveRegistryFilter.sol/NaiveRegistryFilter.json(1 hunks)packages/enclave-contracts/contracts/interfaces/IEnclave.sol(3 hunks)packages/enclave-contracts/scripts/deployEnclave.ts(1 hunks)templates/default/enclave.config.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- examples/CRISP/Cargo.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-09-26T03:11:29.311Z
Learnt from: ryardley
PR: gnosisguild/enclave#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:
crates/support/methods/guest/Cargo.toml
🧬 Code graph analysis (1)
crates/evm-helpers/src/contracts.rs (1)
packages/enclave-sdk/src/contract-client.ts (3)
publishInput(183-223)publishCiphertextOutput(229-272)getE3(278-295)
🪛 dotenv-linter (3.3.0)
examples/CRISP/client/.env.example
[warning] 4-4: [UnorderedKey] The VITE_E3_PROGRAM_ADDRESS key should go before the VITE_ENCLAVE_API key
(UnorderedKey)
[warning] 4-4: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
examples/CRISP/server/.env.example
[warning] 16-16: [UnorderedKey] The E3_PROGRAM_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
🪛 GitHub Actions: Build Crisp Server
crates/support/Dockerfile
[error] 119-125: Cargo build failed due to lock file update requirement while building the server binary. The command failed: 'cargo build --locked --release --bin server'. See log for details.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build_sdk
- GitHub Check: build_enclave_cli
- GitHub Check: integration_prebuild
- GitHub Check: test_net
- GitHub Check: test_contracts
- GitHub Check: build_e3_support_risc0
- GitHub Check: rust_unit
- GitHub Check: Build & Push Image
🔇 Additional comments (27)
packages/enclave-contracts/artifacts/contracts/registry/NaiveRegistryFilter.sol/NaiveRegistryFilter.json (1)
308-308: Build artifact metadata update.The
buildInfoIdchange reflects recompilation of the contract. This is expected when Solidity source files or compiler settings are updated.packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json (1)
222-222: Build artifact metadata update.The
buildInfoIdchange reflects recompilation of the contract. This is expected when Solidity source files or compiler settings are updated.packages/enclave-contracts/scripts/deployEnclave.ts (1)
72-78: Improved deployment output formatting.The consolidated logging output improves readability by grouping all deployment addresses under a single "Deployments" section. All three addresses are still being logged as before.
packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json (1)
707-707: Build artifact metadata update.The
buildInfoIdchange reflects recompilation after the IEnclave interface updates (memory→calldata, getRoot→getInputRoot).crates/evm-helpers/src/contracts.rs (6)
78-78: LGTM! Gas optimization with calldata.The change from
bytes memorytobytes calldatafor thepublicKeyparameter is a gas optimization for external functions, as calldata avoids copying data to memory.
80-80: LGTM! Gas optimization with calldata.The change from
bytes memorytobytes calldatafor thedataparameter improves gas efficiency.
81-81: LGTM! Gas optimization with calldata.Both parameters (
ciphertextOutputandproof) correctly usebytes calldatafor gas efficiency.
84-84: LGTM! Method renamed for clarity.The rename from
getRoottogetInputRootprovides better clarity about what root is being retrieved (the input merkle tree root).
104-104: LGTM! Rust trait updated consistently.The trait method renamed from
get_roottoget_input_rootcorrectly mirrors the Solidity interface change.
316-318: LGTM! Implementation updated consistently.The implementation correctly:
- Updates the method name to
get_input_root- Calls the renamed Solidity function
getInputRootThis maintains consistency with the Solidity interface changes.
packages/enclave-contracts/contracts/interfaces/IEnclave.sol (3)
141-141: LGTM! Gas optimization with calldata.Changing from
bytes memorytobytes calldatafor external function parameters reduces gas costs by avoiding unnecessary memory copies. This is a best practice for external functions that don't modify the parameter data.
163-164: LGTM! Gas optimization with calldata.Both
ciphertextOutputandproofparameters correctly usebytes calldatato optimize gas usage in this external function.
175-176: LGTM! Gas optimization with calldata.Both
plaintextOutputandproofparameters correctly usebytes calldatato optimize gas usage in this external function.examples/CRISP/.gitignore (1)
49-49: LGTM!Adding
.enclave/caches/to the ignore list is consistent with the existing pattern for other.enclave/subdirectories and appropriately excludes generated cache files from version control.templates/default/enclave.config.yaml (1)
13-13: LGTM!The inline comment clarifies the behavior of the
risc0_dev_modeflag, improving documentation for users configuring the system.examples/CRISP/enclave.config.yaml (1)
12-12: LGTM!The inline comment consistently documents the
risc0_dev_modeflag behavior across configuration files.examples/CRISP/contracts/Mocks/MockCRISPInputValidator.sol (2)
15-27: LGTM!The contract structure is clean with appropriate state variables and custom errors. The use of
Clonepattern for initialization is a good practice for factory-deployed contracts.
29-36: LGTM!The initialization logic correctly decodes the appended configuration data and sets up the policy and verifier instances.
examples/CRISP/server/.env.example (1)
16-16: LGTM!The E3_PROGRAM_ADDRESS update aligns with the new ImageID.PROGRAM_ID constant value introduced in this PR. The address consistency across client, server, and contract deployments ensures proper integration.
examples/CRISP/client/.env.example (1)
4-4: LGTM!The E3_PROGRAM_ADDRESS update maintains consistency with the server configuration and the new ImageID.PROGRAM_ID value.
examples/CRISP/deploy/Deploy.s.sol (4)
35-36: LGTM!The imports for both real and mock input validators are properly added to support the conditional deployment logic.
184-194: LGTM!The conditional deployment logic cleanly selects between mock and real validators based on the
USE_MOCK_INPUT_VALIDATORenvironment variable, with appropriate logging for each case.
215-215: LGTM!The update from
ImageID.VOTING_IDtoImageID.PROGRAM_IDaligns with the naming convention changes in the ImageID library for RISC0 v3.
196-202: CRISPInputValidatorFactory constructor accepts address parameter
Constructor inexamples/CRISP/contracts/CRISPInputValidatorFactory.solisconstructor(address inputValidator) Factory(inputValidator) {}which matches the deployed call. No changes needed.
crates/support/contracts/ImageID.sol (1)
22-22: Approve PROGRAM_ID update: consistency verifiedThe new PROGRAM_ID is now consistent across all ImageID.sol definitions (crates/support/contracts, examples/CRISP/contracts, templates/default/contracts).
examples/CRISP/contracts/CRISPInputValidatorFactory.sol (1)
15-15: Deployment scripts updated to new constructor signature AllCRISPInputValidatorFactoryinstances now receiveinputValidatorAddress.examples/CRISP/contracts/ImageID.sol (1)
22-25: No leftover VOTING_ID references detected. All Solidity files now referencePROGRAM_ID.
* chore: update zkvm to v3.0.0 (#791) * chore: update zkvm to v3.0.0 * chore: update deployment script (cherry picked from commit 2d9133e) * chore: update risc0 version v3.0.3 (#793) * chore: update risc0 version * chore: remove risc0 crates from CRISP (cherry picked from commit c94f5f3) * chore: update lock and Dockerfile (#794) (cherry picked from commit c2d7827) * fix: risc0 dev mode env var (#810) * fix: risc0 dev mode env var * chore: lint * chore: add dev comment (cherry picked from commit 7e09bad) * chore: allow mock input validator in CRISP (#782) (cherry picked from commit 2861d9b) * chore: update cargo toml and contract --------- Co-authored-by: ctrlc03 <93448202+ctrlc03@users.noreply.github.com> Co-authored-by: гλ <contact@rudiyardley.com>
3 participants
Summary by CodeRabbit
New Features
Refactor
Chores
Documentation
Bug Fixes