Skip to content

p2p testing#12

Merged
Mehd1b merged 7 commits into
mainfrom
dev
Jan 24, 2026
Merged

p2p testing#12
Mehd1b merged 7 commits into
mainfrom
dev

Conversation

@Mehd1b

@Mehd1b Mehd1b commented Jan 24, 2026

Copy link
Copy Markdown
Member

Summary

  • Add methods crate for building zkVM guest ELF and exporting ZKVM_GUEST_ELF / ZKVM_GUEST_ID
  • Add zkvm-guest wrapper crate for risc0-build compatibility
  • Add e2e-tests crate with feature-gated proof generation tests
  • Add agent code hash binding (build.rs generates AGENT_CODE_HASH at compile time)
  • Add spec/e2e-tests.md specification document
  • Update README with E2E testing and on-chain verification sections

Test Cases

Test Description
test_e2e_success_with_echo Happy path with Groth16 proof generation
test_e2e_agent_code_hash_mismatch Security - wrong hash aborts execution
test_e2e_empty_output Empty output commitment verification
test_e2e_determinism Same input produces identical journals

On-Chain Verification Data

Extracts data needed for Solidity verifier:

  • seal (256 bytes Groth16 proof)
  • journal (KernelJournalV1 bytes)
  • imageId (bytes32 guest identity)

Test Plan

  • cargo test -p e2e-tests passes (unit tests, no RISC Zero required)
  • cargo test -p host-tests passes (92 tests, no regressions)
  • cargo test -p e2e-tests --features risc0-e2e test_e2e_success_with_echo generates valid proof
  • cargo test -p e2e-tests --features risc0-e2e test_e2e_agent_code_hash_mismatch correctly fails
  • cargo test -p e2e-tests --features risc0-e2e test_e2e_empty_output generates valid proof

## Summary

Replace the hardcoded TrivialAgent::run() call in kernel-guest with
the canonical extern "Rust" fn agent_main(ctx, opaque_inputs) entrypoint.

## Key Changes

### API Design
- Use `extern "Rust"` instead of `extern "C"` for ABI safety with Rust types
- 2-argument signature: `agent_main(ctx: &AgentContext, opaque_inputs: &[u8])`
- AgentContext uses owned fields (no lifetimes) - clean "header" struct

### New Crate: example-agent
- Minimal example agent demonstrating the canonical entrypoint
- Echoes input when opaque_inputs[0] == 1, empty output otherwise
- Used for testing kernel execution flow

### Refactored Crates
- kernel-sdk: AgentContext now uses owned [u8; 32] fields, 144 bytes fixed size
- kernel-guest: Uses extern "Rust" block, no unsafe pointer casts needed
- agent-traits: Re-exports from kernel-sdk, deprecated legacy Agent trait
- host-tests: Added tests for agent_main behavior

## Technical Details

- AgentContext is #[repr(C)], Clone, Copy, Debug, PartialEq, Eq
- #[no_mangle] requires #[allow(unsafe_code)] due to Rust lint rules
- All 88 host-tests pass, plus crate-specific tests
…ypoint

feat: Replace TrivialAgent with canonical agent_main entrypoint
The agent-traits crate was providing duplicate ABI definitions that
conflicted with kernel-sdk's canonical definitions. Since nothing is
deployed yet, removing the crate entirely simplifies the architecture.

All canonical ABI types (AgentContext, AgentOutput, AgentEntrypoint)
are now defined solely in kernel-sdk. The example-agent crate
demonstrates the correct agent implementation pattern.

Also fixed stale comment in host-tests that referenced "extern C"
when it should say "extern Rust".
…rypoint

refactor: Remove agent-traits crate and simplify ABI definitions
- Add methods crate for building zkVM guest ELF and exporting IMAGE_ID
- Add zkvm-guest wrapper crate for risc0-build compatibility
- Add e2e-tests crate with feature-gated proof generation tests
- Add agent code hash binding (build.rs generates AGENT_CODE_HASH)
- Add spec/e2e-tests.md specification document
- Update README with E2E testing and on-chain verification sections

Test cases:
- test_e2e_success_with_echo: Happy path with proof generation
- test_e2e_agent_code_hash_mismatch: Security validation
- test_e2e_empty_output: Empty output commitment verification
- test_e2e_determinism: Deterministic execution verification

On-chain data extraction:
- seal (256 bytes Groth16 proof)
- journal (KernelJournalV1 bytes)
- imageId (bytes32 guest identity)
feat: Add E2E zkVM proof tests with RISC Zero integration
@claude

claude Bot commented Jan 24, 2026

Copy link
Copy Markdown

Claude finished @Mehd1b's task —— View job


PR Review: p2p testing

I've completed a comprehensive review of this substantial PR that adds zkVM proof generation and E2E testing infrastructure. This is an impressive implementation with strong security properties and thorough testing.

Todo List

  • Read and understand the overall architecture changes
  • Review the new crates structure (methods, zkvm-guest, e2e-tests, example-agent)
  • Analyze zkVM integration and proof generation logic
  • Review security considerations in guest code
  • Evaluate test coverage and testing approach
  • Check build system and dependency management
  • Assess code quality and best practices
  • Provide comprehensive feedback

Strengths

Architecture & Design

  • Excellent separation of concerns: The 4-crate structure (methods, zkvm-guest, e2e-tests, example-agent) is clean and logical
  • Smart zkVM wrapper design: The zkvm-guest standalone wrapper around kernel-guest elegantly solves risc0-build requirements
  • Robust security model: Agent code hash binding prevents malicious substitution attacks
  • Feature gating: risc0-e2e feature allows CI to run without RISC Zero toolchain

Security Implementation

  • Strong agent binding: AGENT_CODE_HASH cryptographically ties proofs to specific agent code
  • Proper error handling: Hash mismatches correctly cause guest panics, preventing invalid proofs
  • No unsafe code: All crates maintain #![forbid(unsafe_code)] except where necessary (agent_main symbol)
  • Deterministic execution: Consistent journal outputs for identical inputs

Test Coverage (180 tests total)

  • Comprehensive E2E scenarios: Success path, hash mismatch, empty output, determinism
  • Proper failure testing: Wrong hash correctly prevents proof generation
  • On-chain data extraction: Properly extracts seal, journal, and imageId for Solidity verification
  • Unit test coverage: Good coverage of individual components

Code Quality

  • Excellent documentation: Clear inline docs, comprehensive specification in spec/e2e-tests.md
  • Consistent error handling: Appropriate use of Result types and expect() with descriptive messages
  • Clean dependency management: Proper optional dependencies and feature gates

⚠️ Issues & Recommendations

Security Concerns

  1. Development-only hash binding (crates/example-agent/build.rs:8-31):

    // Current: SHA256(src/lib.rs || 0x00 || Cargo.toml)
    // Issue: Source-based hashing is fragile for production
    • Impact: Whitespace or comment changes invalidate proofs
    • Recommendation: Document migration path to artifact-based hashing for production
  2. Panic-based error handling (crates/methods/zkvm-guest/src/main.rs:30):

    panic!("Kernel execution failed: {:?}", error);
    • Risk: Potential information leakage in error messages
    • Recommendation: Consider sanitized error reporting for production

Performance Considerations

  1. Memory usage in parallel testing (crates/e2e-tests/src/lib.rs:74-76):

    • Issue: Parallel proof generation can cause OOM
    • Current mitigation: --test-threads=1 documented
    • Recommendation: Consider proof generation pooling for CI
  2. Build dependency tracking (crates/methods/build.rs:26-58):

    println!("cargo:rerun-if-changed={}", example_agent_dir.join("src").display());
    • Issue: Rebuilds entire zkVM guest on any agent change
    • Optimization: Could be more granular for faster iteration

Code Quality Issues

  1. Inconsistent error expectations (crates/e2e-tests/src/lib.rs:241-245):

    assert!(
        err_string.contains("panic")
            || err_string.contains("failed") 
            || err_string.contains("execution"),
    • Issue: Brittle error message checking
    • Recommendation: Use more specific error patterns
  2. Missing integration docs in main README:

    • Gap: Limited guidance on integrating new agents
    • Recommendation: Add agent development workflow section

Build System

  1. RISC Zero version pinning (multiple Cargo.toml files):
    risc0-zkvm = { version = "3.0", default-features = false }
    • Risk: RISC Zero 3.0 is rapidly evolving
    • Recommendation: Consider pinning to specific patch versions

🔧 Minor Improvements

  1. Code documentation (crates/kernel-guest/src/lib.rs:61-63):

    extern "Rust" {
        fn agent_main(ctx: &AgentContext, opaque_inputs: &[u8]) -> AgentOutput;
    }
    • Enhancement: Document ABI stability requirements more clearly
  2. Test organization (crates/e2e-tests/src/lib.rs:98-354):

    • Suggestion: Consider splitting large test module into separate files

📋 Performance & Quality Metrics

  • Test Count: 180 tests (excellent coverage)
  • Crate Structure: Well-organized 4-crate addition
  • Security Features: Agent code hash binding, no unsafe code
  • Documentation: Comprehensive specs and inline docs
  • Feature Gating: Proper conditional compilation

🎯 Conclusion

This is a high-quality, well-architected implementation that successfully adds zkVM proof generation and E2E testing. The security model is sound, test coverage is comprehensive, and the code quality is excellent.

Recommendation: Approve with minor follow-ups

The identified issues are primarily future considerations rather than blocking problems. The development-time source hashing is clearly documented as temporary, and the architecture supports smooth migration to production artifact hashing.

The E2E testing framework provides exactly what's needed for proving kernel execution correctness and will be valuable for ongoing development.


@Mehd1b
Mehd1b merged commit 2e5c1a4 into main Jan 24, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant