Feat/ef io interface

[jotabulacios]

This PR adds the EF zkVM IO interface:
https://github.com/eth-act/zkvm-standards/blob/main/standards/io-interface/README.md

• Adds read_input / write_output C shims.
• read_input returns a zero-copy pointer + size over the memory-mapped private input region.
• write_output maps to the Commit ECALL and now concatenates across multiple calls.
• Caps total public output at 1 MB across calls.
• Changes load_bytes to return an error on address overflow instead of panicking on guest-controlled
  input.

Includes executor/prover coverage for the EF IO demo and multi-call output concatenation

BREAKING: the Commit ECALL now concatenates across calls instead of overwriting. This aligns the executor with the EF zkVM IO interface spec

[claude]

PR Review: Feat/ef io interface

Summary

Clean implementation of the EF zkVM IO interface. The append semantics for commit_public_output are a clear improvement over the old overwrite model, and test coverage is good (unit tests + integration + prover e2e). Two low-severity issues worth fixing.

---

Low: load_bytes wrapping arithmetic in commit_public_output

In load_bytes (memory.rs), let end = addr + len uses plain addition. If a guest provides a1 (output pointer) near u64::MAX, the addition wraps in release mode, causing end < addr — the loop skips entirely and commit_public_output returns Ok(()) while appending zero bytes, but the size counter was already incremented. The 64 KB cap bounds len but not addr.

load_bytes predates this PR, but the path ECALL → commit_public_output → load_bytes is new, and the guest controls both values. Fix: use addr.saturating_add(len) or a checked_add guard.

---

Low: #[unsafe(no_mangle)] on non-riscv64 stubs

The non-riscv64 stubs for read_input / write_output in ef_io.rs export into the global C symbol namespace even in host/test builds. These are extremely generic names; any transitive C dependency that also defines them will cause a silent linker conflict. Stubs that immediately unimplemented!() have no reason to be in the C namespace — removing #[unsafe(no_mangle)] from the non-riscv64 variants eliminates the risk at no cost. The riscv64 variants must keep it for EF spec compliance.

[github-actions]

Codex Code Review

No issues found in the PR diff.

I attempted verification, but cargo check could not run in this environment because rustup/cargo needed to write under read-only home directories and fetch network dependencies.

[github-actions]

Benchmark Results for modified programs 🚀

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
head hashmap	133.8 ± 4.4	129.1	144.1	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
head keccak	128.9 ± 1.7	127.4	133.6	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
head syscall_commit	90.6 ± 0.8	89.8	92.2	1.00

[github-actions]

Benchmark Results for unmodified programs 🚀

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
base binary_search	78.1 ± 12.8	59.0	91.3	1.24 ± 0.25
head binary_search	62.8 ± 7.1	58.1	77.4	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
base bitwise_ops	59.9 ± 1.6	58.7	63.1	1.02 ± 0.04
head bitwise_ops	58.5 ± 1.7	56.9	61.8	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
base fibonacci_26	63.9 ± 1.1	62.7	65.9	1.00
head fibonacci_26	67.2 ± 9.3	61.6	93.1	1.05 ± 0.15

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
base matrix_multiply	63.6 ± 0.7	62.3	64.8	1.01 ± 0.02
head matrix_multiply	63.0 ± 0.8	61.9	64.2	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
base modular_exp	59.8 ± 0.4	59.4	60.8	1.01 ± 0.02
head modular_exp	59.2 ± 1.1	58.1	61.7	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
base quicksort	63.6 ± 1.1	62.6	65.4	1.01 ± 0.02
head quicksort	62.8 ± 0.9	61.5	64.6	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
base sieve	65.4 ± 0.5	64.8	66.0	1.00 ± 0.02
head sieve	65.1 ± 0.9	64.2	67.0	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
base sum_array	91.1 ± 12.9	73.9	104.0	1.23 ± 0.17
head sum_array	74.0 ± 0.8	72.7	75.5	1.00

[github-actions]

Codex Code Review

Finding

• Medium Security / Bug - executor/src/vm/memory.rs:194 allocates Vec::with_capacity(len as usize) before any fallible size handling. len is guest-controlled for Print and Panic ecalls via execution.rs:307 and execution.rs:316. A guest can pass a huge length and panic/abort the host with capacity overflow or OOM instead of returning MemoryError. Move the address overflow check before allocation and reject or fallibly reserve large lengths, e.g. try_reserve_exact, ideally with a bounded max for non-commit byte loads.

Verification

I attempted cargo test -p executor vm::memory --lib, but it could not run because rustup tried to write under /home/runner/.rustup/tmp and the filesystem is read-only.

[claude]

Review: Feat/ef io interface

Good overall. The EF IO standard mapping is clean, the append-semantics change is correct, and fixing load_bytes to return Result instead of panicking on overflow is a real improvement. Tests cover the key cases.

Two issues found:

Medium – OOM in load_bytes for PRINT/PANIC syscalls

Vec::with_capacity(len as usize) runs before the checked_add guard. The commit_public_output path is safe (1 MB cap enforced first), but the PRINT and PANIC syscall handlers pass registers.read(11)? (fully guest-controlled, up to u64::MAX) directly into load_bytes with no size bound. A guest that sets a1 = 1_000_000_000 causes a ~1 GB allocation and host process abort.

The PR description calls out fixing "panicking on guest-controlled input" — this is the remaining case. Either add a small cap in the PRINT/PANIC handlers (e.g. 64 KB), or move the with_capacity hint after checked_add and add a max-length guard.

See inline comment on executor/src/vm/memory.rs:196.

Low – Redundant nested unsafe blocks in read_input

Two inner unsafe { … } blocks inside an already-unsafe fn add noise and imply a false isolation. The outer unsafe fn already opts in to all unsafe operations.

See inline comment on syscalls/src/ef_io.rs:36–41 (has a code suggestion).

---

Everything else looks good:

• Append semantics + running total cap is the right model for multi-call output.
• checked_add fix for addr overflow is correct.
• Memory struct refactor to named fields is a clean improvement.
• Test coverage for the new append/cap/overflow behaviour is solid.

[jotabulacios]

/claude /codex

[github-actions]

Codex Code Review

No issues found in the PR diff.

I attempted targeted checks, but they could not run in this environment because rustup needs to write under /home/runner/.rustup/tmp, which is read-only:

error: could not create temp file ... Read-only file system

Residual risk: I did not verify compilation/tests locally due that toolchain setup failure.

[claude]

Review: Feat/ef io interface

Overall this is a well-structured PR. The safety hardening in load_bytes (overflow-checked address arithmetic, try_reserve_exact instead of Vec::with_capacity) is the right direction, and the new tests cover the important edge cases. A few issues to address:

---

Medium

Breaking semantic change — overwrite to append in commit_public_output
The executor previously kept only the last commit call's value; it now concatenates all calls. Any existing guest that calls commit more than once expecting the final call to win will silently produce a wrong (concatenated) output with no error. This is intentional for the EF IO spec, but needs to be called out explicitly — a BREAKING CHANGE: note in the PR description, or a deprecation comment on the old commit syscall helper, so integrators know to audit existing guests. See inline comment on memory.rs.

read_input exposes an unchecked host-supplied length
read_volatile returns whatever the host wrote at PRIVATE_INPUT_START. The value is not validated against MAX_PRIVATE_INPUT_SIZE before being handed to the caller as buf_size. A buggy host could write a length implying data outside the mapped region; guest code passing buf_size straight to write_output would then hit the total-output cap deep inside the executor rather than failing early. Clamp/validate the length in read_input. See inline comment on ef_io.rs.

len as usize can truncate silently on 32-bit hosts
In the new load_bytes, try_reserve_exact(len as usize) truncates u64 to usize on 32-bit platforms. For example len = 2^32 + 1 becomes 1, passes the allocation check, and only reads 1 byte instead of erroring. Use usize::try_from(len).map_err(|_| MemoryError::AllocationFailed)? instead. See inline comment on memory.rs.

---

Low

Redundant nested unsafe blocks in ef_io.rs
Both read_input and write_output wrap their body in unsafe { ... } inside an already-unsafe fn. The inner block is a no-op; the existing commit in syscalls.rs does not do this. See inline comments on ef_io.rs.

public_output.len() as u64 cast in total-cap check
For consistency with the rest of the safety hardening, prefer u64::try_from(self.public_output.len()).unwrap_or(u64::MAX) over a bare as u64 cast. See inline comment on memory.rs.

SyscallNumbers made pub without doc or semver note
Making a previously-private enum pub is a public API addition. Worth a short doc comment explaining that this is an implementation detail exposed for ef_io, not a stable API.

---

Positive observations

• MAX_PUBLIC_OUTPUT_TOTAL_SIZE = 1 MB is well-documented; renaming from the old TODO-marked MAX_PUBLIC_OUTPUT_COMMIT_SIZE = 1024 is a clear improvement.
• Memory refactored from a newtype to a named-field struct is cleaner; the public_output: Vec<u8> field correctly separates the output buffer from addressable memory.
• checked_add guard before computing end in load_bytes correctly closes the original panic-on-guest-controlled-overflow bug.
• Test coverage for address overflow, huge allocation, and the total-cap boundary is thorough.

[jotabulacios]

/claude

[claude]

Review: Feat/ef io interface

Overall: Clean, well-scoped PR. The security fixes to load_bytes (OOM/overflow → Err instead of panic/abort) and moving public output out of guest memory are clear improvements. Two findings worth addressing:

---

Medium — AllocationFailed conflates two distinct error conditions

In the new load_bytes:

let len_usize = usize::try_from(len).map_err(|_| MemoryError::AllocationFailed)?;
// ...
result.try_reserve_exact(len_usize).map_err(|_| MemoryError::AllocationFailed)?;

The first arm is an architecture-mismatch check (unreachable on 64-bit hosts); the second is a real OOM. Using the same variant makes it impossible to distinguish them in error logs. A separate MemoryError::LengthTooLarge for the try_from branch would help diagnosis.

---

Low — debug_assert_eq is stripped in release builds

debug_assert_eq!(
    traces.public_output_bytes, result.return_values.memory_values,
    "public output diverged between executor view and trace reconstruction"
);

In production (release) proving runs this check is a no-op. If the executor and trace builder ever diverge on public output, a wrong proof is silently produced. The COMMIT AIR bus-balance check catches it at verification time, but that requires a full verify round-trip. Consider returning Err here so a production prover surfaces the inconsistency immediately rather than handing the caller a proof that will fail to verify.

---

Noted (no action needed)

• Breaking change (commit concatenates instead of overwrites) is clearly documented.
• store_private_inputs early-return on empty input leaves the length word at zero — read_input reads it as 0, sets buf_size = 0, and the demo guards with if buf_size > 0. Consistent.
• Double unsafe {} in ef_io.rs is required by Rust 2024 edition — not a bug.