Update CLI features and ZKVM syscall interface#18
Update CLI features and ZKVM syscall interface#18this-vishalsingh wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR mostly applies formatting/structure cleanups across the zkVM guest library exports and the CLI command implementation, aligning import/export ordering and reformatting longer match arms and print statements for readability.
Changes:
- Reformat and lightly reorganize zkVM guest crate modules/exports (
lib.rs,prelude.rs) and syscall wrappers (syscalls.rs). - Reformat zkVM guest I/O syscall wrappers (
io.rs) without changing intended behavior. - Reformat the CLI command dispatch and output formatting, including Ethereum-related commands (
crates/cli/src/main.rs).
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/zkvm/src/syscalls.rs | Whitespace/formatting adjustments around syscall wrappers. |
| crates/zkvm/src/prelude.rs | Reorders prelude re-exports. |
| crates/zkvm/src/lib.rs | Reorders module declarations and re-exports; formatting in entry macro. |
| crates/zkvm/src/io.rs | Whitespace/formatting adjustments around guest I/O syscalls. |
| crates/cli/src/main.rs | Reformat CLI imports, match arms, and output formatting; touches Ethereum CLI commands. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Compute Blake2b hash (64-byte output) | ||
| pub fn blake2b(data: &[u8]) -> [u8; 64] { | ||
| let output = [0u8; 64]; | ||
|
|
||
| #[cfg(target_arch = "riscv32")] | ||
| { | ||
| unsafe { |
There was a problem hiding this comment.
In blake2b, output is declared immutable but later used with as_mut_ptr() / copy_from_slice(), which won’t compile (and would be unsound if it did). Declare output as mutable so both the syscall path and testing path can write into it.
| unsafe { | ||
| core::arch::asm!( | ||
| "li a7, 0x02", // READ syscall | ||
| "li a0, 0", // NULL pointer = query size | ||
| "li a1, 0", // max_size = 0 for query | ||
| "ecall", | ||
| "mv {size}, a0", | ||
| size = out(reg) size, |
There was a problem hiding this comment.
The inline asm! blocks write to fixed registers (a0/a1/a7) but don’t declare them as inputs/outputs/clobbers. This is undefined behavior because the compiler may keep live values in those registers across the asm block. Use explicit register operands (e.g., in("a7"), in("a0"), in("a1"), lateout("a0")) and/or clobber_abi("C") to make register usage explicit.
| println!(" Blocks: {} to {}", from, to); | ||
| println!(" Total proofs: {}", proofs.len()); | ||
| println!(" Time: {:?}", elapsed); | ||
| println!(" Avg per block: {:?}", elapsed / proofs.len() as u32); | ||
| } |
There was a problem hiding this comment.
elapsed / proofs.len() as u32 will panic if proofs is empty (e.g., when from > to, since from..=to produces no items). Add validation for from <= to and/or guard the average computation when proofs.is_empty().
| @@ -301,50 +361,57 @@ fn main() { | |||
| async fn prove_tx_command(rpc_url: &str, tx_hash_str: &str, output_dir: &str) { | |||
| println!("🔗 Proving Ethereum transaction: {}", tx_hash_str); | |||
| println!(" RPC: {}", rpc_url); | |||
There was a problem hiding this comment.
prove_tx_command takes output_dir but never uses it, which will trigger an unused-variable warning and makes the CLI behavior unclear (the command accepts --output-dir but doesn’t write anything). Either use the directory (or at least log it), or rename the parameter to _output_dir until it’s implemented.
| println!(" RPC: {}", rpc_url); | |
| println!(" RPC: {}", rpc_url); | |
| println!(" Output: {}", output_dir); |
| pub fn ecrecover(hash: &[u8; 32], v: u8, r: &[u8; 32], s: &[u8; 32]) -> Option<[u8; 20]> { | ||
| let address = [0u8; 20]; | ||
|
|
||
| #[cfg(target_arch = "riscv32")] |
There was a problem hiding this comment.
ecrecover passes address.as_ptr() into the syscall, but address is an immutable local. If the ecall writes to that pointer, this violates Rust aliasing rules and the optimizer may treat address as never-modified (leading to always returning zero address). Make address mutable (or use MaybeUninit) and pass a mutable pointer so the syscall output is defined and observable.
No description provided.