[Feat] Staff for running zkvm prover #1666
Conversation
Signed-off-by: noelwei <[email protected]>
Signed-off-by: noelwei <[email protected]>
Signed-off-by: noelwei <[email protected]>
WalkthroughThis update introduces several new configuration files and scripts to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Makefile
participant DownloadScript
participant S3Bucket
User->>Makefile: make test_run
Makefile->>DownloadScript: .work/download-release.sh
DownloadScript->>S3Bucket: Request app.vmexe files (chunk, batch, bundle)
S3Bucket-->>DownloadScript: Return binaries
DownloadScript->>Makefile: Binaries saved in .work/
Makefile->>User: Run cargo with config.json and environment variables
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Rename config.json to template Signed-off-by: noelwei <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
zkvm-prover/.work/download-release.sh (2)
13-14: Same directory prep needed for batch circuitSince this block writes to
.work/batch, please ensure:+ mkdir -p .work/batchbefore downloading.
16-17: Same directory prep needed for bundle circuitEnsure
.work/bundleexists by adding:+ mkdir -p .work/bundleprior to the download.
🧹 Nitpick comments (9)
zkvm-prover/README.md (2)
7-7: Fix typo in 'receving'The word “receving” should be corrected to “receiving.”
9-9: Clarify copy ofconfig.jsonfrom templateConsider rephrasing to: “Create a
config.jsonin this directory by copying fromconfig.json.template.” for clarity.zkvm-prover/.work/download-release.sh (1)
8-8: Improve version echo outputPrefix the version message for clarity:
- echo $SCROLL_ZKVM_VERSION + echo "Using release version: $SCROLL_ZKVM_VERSION"zkvm-prover/.work/batch/openvm.toml (1)
7-16: Provide context for empty VM module tables.
Several VM modules (rv32i,rv32m,io,keccak,castf,native,sha256) are declared without inner parameters. If leaving them blank is intentional (using defaults), consider adding an inline comment like# using defaultsto clarify this for future maintainers.zkvm-prover/Makefile (3)
8-12: Simplify Plonky3 version extraction.
The conditional handling of#escaping based on Make version is brittle. Consider a single command that works uniformly, e.g.:PLONKY3_VERSION := $(shell grep -m1 'Plonky3.git' Cargo.lock | cut -d# -f2 | cut -c1-7)
55-57: Ensure idempotent downloads.
The rule for$(CIRCUIT_STUFF)always invokesdownload-release.sh, even if binaries exist. To avoid redundant downloads, add a check inside the script or adjust the Make rule to run only when a target is missing or outdated.
58-59: Check forconfig.jsonbefore running.
Thetest_runtarget expects./config.jsonbut doesn’t verify its presence. For better UX, add a prerequisite or guard clause that prompts users to copy fromconfig.json.templateif the file is missing.zkvm-prover/.work/chunk/openvm.toml (2)
7-12: Document empty VM module tables.
Tablesrv32i,io, andkeccakare empty. If defaults are intended, add comments (e.g.,# using defaults) to avoid confusion.
35-36: Emptysha256table.
If no parameters are needed for SHA-256, add a comment for clarity (e.g.,# defaults).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
zkvm-prover/.work/.gitignore(1 hunks)zkvm-prover/.work/batch/openvm.toml(1 hunks)zkvm-prover/.work/bundle/openvm.toml(1 hunks)zkvm-prover/.work/chunk/openvm.toml(1 hunks)zkvm-prover/.work/download-release.sh(1 hunks)zkvm-prover/Makefile(2 hunks)zkvm-prover/README.md(1 hunks)zkvm-prover/config.json.template(2 hunks)
🧰 Additional context used
🪛 LanguageTool
zkvm-prover/README.md
[uncategorized] ~3-~3: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...cate with coordinator, receving proving task and generate proof ## Testing + Get t...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: tests
🔇 Additional comments (24)
zkvm-prover/.work/.gitignore (1)
1-1: Appropriate.gitignoreentry for VM executablesThis rule correctly prevents downloaded
.vmexebinaries from being tracked in git, avoiding large binary commits.zkvm-prover/README.md (1)
11-11: Documentmake test_runcommandThe instruction to run
make test_runis clear and concise.zkvm-prover/.work/bundle/openvm.toml (1)
1-17: Verify empty VM config sectionsAll VM module sections (
rv32i,rv32m,io,keccak,castf,native) are intentionally left empty here. Please confirm that this minimal configuration is valid for the bundle circuit.zkvm-prover/config.json.template (5)
3-4: Update default prover naming and key directoryChanging the prefix to “test-prover” and standardizing
keys_dirto.workaligns the template with the new directory structure.
6-6: Use placeholder for coordinator URLReplacing the actual URL with
<the url of coordinator>makes the template environment-agnostic.
12-12: Use placeholder for RPC endpointThe
<the url of rpc endpoint>placeholder is appropriate for users to fill in their chain endpoint.
23-23: Standardize database pathSetting
"db_path": ".work/db"ensures the database files reside under the new.workdirectory.
28-28: Standardize workspace pathChanging the
workspace_pathto.workmatches the rest of the configuration.zkvm-prover/.work/download-release.sh (1)
19-20: Legacy download fallback is acceptableThe
|| echopattern handles missing legacy binaries gracefully.zkvm-prover/.work/batch/openvm.toml (5)
1-5: Validate FRI parameters.
The[app_fri_params.fri_params]block sets critical proving parameters (blow‐up factor, polynomial length, query count, PoW bits). Ensure these match the batch circuit’s security and performance requirements, particularly thatnum_queries = 100andproof_of_work_bits = 16are sufficient for your test scenarios.
17-21: Supported modular arithmetic moduli.
Thesupported_modulusentries under[app_vm_config.modular]appear correct for field operations. Verify that these prime moduli align exactly with the batch circuit’s expected fields.
23-25: Supported pairing curves.
supported_curves = ["Bls12_381"]is the standard choice for many zk circuits. Confirm that no additional curves are required for the batch circuit.
27-29: Supported extension field moduli.
The FP2 modulus matches the primary modular entry. Ensure that FP2 operations exclusively use this base field and that no extra moduli are required.
30-34: Elliptic curve parameters.
The[[app_vm_config.ecc.supported_curves]]block defines a curve withb = "4", which matches the Bls12-381 curve parameters. Double-check that these coefficients are correct for your use case.zkvm-prover/Makefile (2)
3-4: Verify the RUST_MIN_STACK usage.
You've setRUST_MIN_STACK ?= 16777216(16 MB) and exported it. Ensure this stack size covers the prover’s worst-case recursion/depth on all target platforms. Document why 16 MB was chosen.
6-6: Group circuit executables neatly.
DefiningCIRCUIT_STUFFto list all three.vmexetargets is clear and maintainable.zkvm-prover/.work/chunk/openvm.toml (8)
1-5: Validate FRI parameters for chunk circuit.
The chunk circuit reuseslog_blowup = 1,log_final_poly_len = 0,num_queries = 100, andproof_of_work_bits = 16. Confirm these choices satisfy the chunk circuit’s soundness/performance trade-offs.
13-17: Consistent range tuple checker sizes.
Both[app_vm_config.rv32m]and[app_vm_config.bigint]setrange_tuple_checker_sizes = [256, 8192]. Verify these values align with the chunk circuit’s witness layout.
19-27: Supported modular arithmetic moduli.
The list of six prime moduli appears comprehensive. Double-check that each modulus is actively used by the chunk circuit and remove any unused entries to keep the list concise.
29-30: Extension field FP2 modulus.
supported_modulus = ["218882...08583"]correctly matches the BN254 base prime. Approve.
32-33: Pairing curve selection.
supported_curves = ["Bn254"]is expected for the chunk circuit’s pairing operations. Approve.
37-42: First ECC curve parameters.
The first[[app_vm_config.ecc.supported_curves]]block usesa = "0", b = "7". Confirm this matches the intended curve (likely secp256k1 or secp256r1).
43-48: Second ECC curve parameters.
The non-zero coefficients in this block define a twisted curve. Ensure these values align with the desired twist group parameters.
49-53: Third ECC curve parameters.
This final curve likely corresponds to BN254. Verify that the modulus, scalar, and curve coefficients are correct for BN254’s short Weierstrass form.
| @@ -0,0 +1,20 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add strict mode to the script
Enable robust error handling by adding:
+ set -euo pipefailimmediately after the shebang.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #!/bin/bash | |
| #!/bin/bash | |
| set -euo pipefail |
🤖 Prompt for AI Agents
In zkvm-prover/.work/download-release.sh at line 1, add strict mode settings
immediately after the shebang line by including 'set -euo pipefail' to enable
robust error handling and make the script exit on errors, unset variables, or
failed pipes.
| # chunk-circuit exe | ||
| wget https://circuit-release.s3.us-west-2.amazonaws.com/scroll-zkvm/releases/$SCROLL_ZKVM_VERSION/chunk/app.vmexe -O .work/chunk/app.vmexe |
There was a problem hiding this comment.
Ensure target directories exist before download
The script writes to .work/chunk but does not create it. Add:
+ mkdir -p .work/chunkbefore the wget command to prevent failures.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # chunk-circuit exe | |
| wget https://circuit-release.s3.us-west-2.amazonaws.com/scroll-zkvm/releases/$SCROLL_ZKVM_VERSION/chunk/app.vmexe -O .work/chunk/app.vmexe | |
| # chunk-circuit exe | |
| mkdir -p .work/chunk | |
| wget https://circuit-release.s3.us-west-2.amazonaws.com/scroll-zkvm/releases/$SCROLL_ZKVM_VERSION/chunk/app.vmexe -O .work/chunk/app.vmexe |
🤖 Prompt for AI Agents
In zkvm-prover/.work/download-release.sh around lines 10 to 11, the script
downloads a file into the .work/chunk directory but does not ensure this
directory exists, which can cause the wget command to fail. Add a command to
create the .work/chunk directory (e.g., mkdir -p .work/chunk) before the wget
command to guarantee the target directory is present before downloading the
file.
| if [ -z "${SCROLL_ZKVM_VERSION}" ]; then | ||
| SCROLL_ZKVM_VERSION=$($SHELL ./print_high_zkvm_version.sh | cut -d' ' -f1|cut -c2-) | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify and harden version detection
Avoid using $SHELL and handle errors if the version script fails. For example:
- if [ -z "${SCROLL_ZKVM_VERSION}" ]; then
- SCROLL_ZKVM_VERSION=$($SHELL ./print_high_zkvm_version.sh | cut -d' ' -f1 | cut -c2-)
- fi
+ if [ -z "${SCROLL_ZKVM_VERSION:-}" ]; then
+ SCROLL_ZKVM_VERSION=$(./print_high_zkvm_version.sh 2>/dev/null | awk '{print substr($1,2)}') \
+ || { echo "Failed to determine SCROLL_ZKVM_VERSION"; exit 1; }
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -z "${SCROLL_ZKVM_VERSION}" ]; then | |
| SCROLL_ZKVM_VERSION=$($SHELL ./print_high_zkvm_version.sh | cut -d' ' -f1|cut -c2-) | |
| fi | |
| if [ -z "${SCROLL_ZKVM_VERSION:-}" ]; then | |
| SCROLL_ZKVM_VERSION=$(./print_high_zkvm_version.sh 2>/dev/null | awk '{print substr($1,2)}') \ | |
| || { echo "Failed to determine SCROLL_ZKVM_VERSION"; exit 1; } | |
| fi |
🤖 Prompt for AI Agents
In zkvm-prover/.work/download-release.sh around lines 4 to 6, the version
detection uses $SHELL to run the script, which is unnecessary and may cause
issues. Replace the use of $SHELL with a direct script invocation, and add error
handling to check if the version script executes successfully. If it fails,
provide a clear error message and exit the script to prevent further errors.
Enable a easy launching tes-running of zkvm prover
This PR add required stuff and scripts, so developer can launch zkvm prover with known endpoint of coordinator and l2geth. For example, we can launch prover and make it receiving proving tasks from seoplia or mainnet for a e2e testing
TODO:
We still need a manually update of openvm config files (openvm.toml) under the directory of circuit apps in case it has changed in the
zkvm-provercrate. Considering we would make more refactors in the near future such a status would not last longSummary by CodeRabbit
New Features
Chores