Conversation
There was a problem hiding this comment.
Pull request overview
Adds SP1 zkVM support to the state-transition runtime by wiring up the SP1 linker script, syscall-backed I/O/input handling, and registering the new target in the build so the runtime can be produced for SP1.
Changes:
- Register
sp1as a zkVM build target (riscv32-freestanding-none). - Implement SP1 runtime syscalls for
halt,get_input, and stdout printing. - Update SP1 linker script to define memory layout and allocator boundary symbols (e.g.
_end,__global_pointer$).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pkgs/state-transition-runtime/src/sp1/sp1.ld |
Aligns SP1 linker script with expected RISC-V freestanding layout; defines RAM/stack regions and _end for the allocator. |
pkgs/state-transition-runtime/src/sp1/lib.zig |
Implements SP1 runtime entry, halt syscall, syscall-based input, and a fixed-buffer allocator based on _end..RAM-end. |
pkgs/state-transition-runtime/src/sp1/io.zig |
Implements print_str via SP1 WRITE syscall to stdout. |
build.zig |
Adds sp1 to the list of zkVM targets built by the project. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn get_input(allocator: std.mem.Allocator) []const u8 { | ||
| // HINT_LEN syscall: returns the input length in t0 | ||
| const len: usize = asm volatile ("ecall" | ||
| : [ret] "={t0}" (-> usize), | ||
| : [syscall_num] "{t0}" (syscalls.HINT_LEN), | ||
| ); | ||
|
|
||
| const input = allocator.alloc(u8, len) catch @panic("could not allocate space for input"); |
There was a problem hiding this comment.
get_input allocates len bytes directly from the HINT_LEN syscall result without any upper bound/sanity check. Since the hint length ultimately comes from external input, this can trigger very large allocations (or an allocator panic/OOM) and make the runtime easy to DoS. Consider enforcing a maximum input size (similar to the 10MB cap in risc0/lib.zig) and failing with a clear panic message if exceeded.
| return input; | ||
| } | ||
|
|
||
| pub fn free_input(_: std.mem.Allocator) void {} |
There was a problem hiding this comment.
free_input is currently a no-op, but get_input allocates a buffer from the provided allocator. If the runtime continues allocating after reading input (e.g. deserialization, logging formatting), keeping the input allocation alive for the whole run can unnecessarily consume the fixed heap and lead to allocation failures for large inputs. Consider either (a) freeing/resetting the allocation in free_input (which may require tracking the allocated slice internally), or (b) changing the interface to accept the input slice so it can be freed explicitly.
No description provided.