From 9100a82e5f2a45a600d47cc0c4273bc201d4eb9a Mon Sep 17 00:00:00 2001 From: nhtyy Date: Thu, 12 Dec 2024 14:58:15 -0800 Subject: [PATCH 1/4] chore(executor): make input stream a queue --- crates/core/executor/src/executor.rs | 5 ++-- crates/core/executor/src/hook.rs | 11 ++++++++ crates/core/executor/src/state.rs | 11 +++----- crates/core/executor/src/syscalls/hint.rs | 31 ++++++++++------------ crates/core/executor/src/syscalls/write.rs | 11 +++++--- 5 files changed, 38 insertions(+), 31 deletions(-) diff --git a/crates/core/executor/src/executor.rs b/crates/core/executor/src/executor.rs index c2cc1915ab..11eb33e914 100644 --- a/crates/core/executor/src/executor.rs +++ b/crates/core/executor/src/executor.rs @@ -1594,8 +1594,9 @@ impl<'a> Executor<'a> { many proofs in or forget to call verify_sp1_proof?" ); } - if self.state.input_stream_ptr != self.state.input_stream.len() { - tracing::warn!("Not all input bytes were read."); + + if !self.state.input_stream.is_empty() { + tracing::warn!("Not all input bytes were read"); } if self.emit_global_memory_events diff --git a/crates/core/executor/src/hook.rs b/crates/core/executor/src/hook.rs index 63645d84d8..eb17a6ed92 100644 --- a/crates/core/executor/src/hook.rs +++ b/crates/core/executor/src/hook.rs @@ -18,9 +18,15 @@ pub type BoxedHook<'a> = Arc>; pub const FD_K1_ECRECOVER_HOOK: u32 = 5; /// The file descriptor through which to access `hook_r1_ecrecover`. pub const FD_R1_ECRECOVER_HOOK: u32 = 6; + +// 7 is used in main + /// The file descriptor through which to access `hook_ed_decompress`. pub const FD_EDDECOMPRESS: u32 = 8; +/// The file descriptor through which to access `hook_rsa_mul_mod`. +pub const FD_RSA_MUL_MOD: u32 = 9; + /// A runtime hook. May be called during execution by writing to a specified file descriptor, /// accepting and returning arbitrary data. pub trait Hook { @@ -223,6 +229,11 @@ pub fn hook_r1_ecrecover(_: HookEnv, buf: &[u8]) -> Vec> { vec![vec![1], bytes.to_vec(), s_inverse.to_bytes().to_vec()] } +#[must_use] +pub fn hook_rsa_mul_mod(_: HookEnv, buf: &[u8]) -> Vec> { + +} + #[cfg(test)] pub mod tests { use super::*; diff --git a/crates/core/executor/src/state.rs b/crates/core/executor/src/state.rs index 55ba22e321..2bc29a57e5 100644 --- a/crates/core/executor/src/state.rs +++ b/crates/core/executor/src/state.rs @@ -1,6 +1,5 @@ use std::{ - fs::File, - io::{Seek, Write}, + collections::VecDeque, fs::File, io::{Seek, Write} }; use hashbrown::HashMap; @@ -42,10 +41,7 @@ pub struct ExecutionState { pub uninitialized_memory: PagedMemory, /// A stream of input values (global to the entire program). - pub input_stream: Vec>, - - /// A ptr to the current position in the input stream incremented by `HINT_READ` opcode. - pub input_stream_ptr: usize, + pub input_stream: VecDeque>, /// A stream of proofs (reduce vk, proof, verifying key) inputted to the program. pub proof_stream: @@ -77,8 +73,7 @@ impl ExecutionState { pc: pc_start, memory: PagedMemory::new_preallocated(), uninitialized_memory: PagedMemory::default(), - input_stream: Vec::new(), - input_stream_ptr: 0, + input_stream: VecDeque::new(), public_values_stream: Vec::new(), public_values_stream_ptr: 0, proof_stream: Vec::new(), diff --git a/crates/core/executor/src/syscalls/hint.rs b/crates/core/executor/src/syscalls/hint.rs index a740250fe9..4946a118ec 100644 --- a/crates/core/executor/src/syscalls/hint.rs +++ b/crates/core/executor/src/syscalls/hint.rs @@ -1,4 +1,5 @@ use super::{Syscall, SyscallCode, SyscallContext}; +use std::collections::VecDeque; pub(crate) struct HintLenSyscall; @@ -10,14 +11,7 @@ impl Syscall for HintLenSyscall { _arg1: u32, _arg2: u32, ) -> Option { - if ctx.rt.state.input_stream_ptr >= ctx.rt.state.input_stream.len() { - panic!( - "failed reading stdin due to insufficient input data: input_stream_ptr={}, input_stream_len={}", - ctx.rt.state.input_stream_ptr, - ctx.rt.state.input_stream.len() - ); - } - Some(ctx.rt.state.input_stream[ctx.rt.state.input_stream_ptr].len() as u32) + Some(next_len_or_panic(&ctx.rt.state.input_stream)) } } @@ -25,18 +19,15 @@ pub(crate) struct HintReadSyscall; impl Syscall for HintReadSyscall { fn execute(&self, ctx: &mut SyscallContext, _: SyscallCode, ptr: u32, len: u32) -> Option { - if ctx.rt.state.input_stream_ptr >= ctx.rt.state.input_stream.len() { - panic!( - "failed reading stdin due to insufficient input data: input_stream_ptr={}, input_stream_len={}", - ctx.rt.state.input_stream_ptr, - ctx.rt.state.input_stream.len() - ); - } - let vec = &ctx.rt.state.input_stream[ctx.rt.state.input_stream_ptr]; - ctx.rt.state.input_stream_ptr += 1; + let _ = next_len_or_panic(&ctx.rt.state.input_stream); + + // SAFTEY: We check if we have a vec in the input stream in the previous line. + let vec = unsafe { ctx.rt.state.input_stream.pop_front().unwrap_unchecked() }; + assert!(!ctx.rt.unconstrained, "hint read should not be used in a unconstrained block"); assert_eq!(vec.len() as u32, len, "hint input stream read length mismatch"); assert_eq!(ptr % 4, 0, "hint read address not aligned to 4 bytes"); + // Iterate through the vec in 4-byte chunks for i in (0..len).step_by(4) { // Get each byte in the chunk @@ -61,3 +52,9 @@ impl Syscall for HintReadSyscall { None } } + +fn next_len_or_panic(queue: &VecDeque>) -> u32 { + queue.front().map(|vec| vec.len() as u32).unwrap_or_else(|| { + panic!("Syscall Hint Len failed becasue the input stream is exhausted"); + }) +} diff --git a/crates/core/executor/src/syscalls/write.rs b/crates/core/executor/src/syscalls/write.rs index d0db44cae8..ba5caf03e5 100644 --- a/crates/core/executor/src/syscalls/write.rs +++ b/crates/core/executor/src/syscalls/write.rs @@ -63,12 +63,15 @@ impl Syscall for WriteSyscall { } else if fd == 3 { rt.state.public_values_stream.extend_from_slice(slice); } else if fd == 4 { - rt.state.input_stream.push(slice.to_vec()); + rt.state.input_stream.push_back(slice.to_vec()); } else if let Some(mut hook) = rt.hook_registry.get(fd) { let res = hook.invoke_hook(rt.hook_env(), slice); - // Add result vectors to the beginning of the stream. - let ptr = rt.state.input_stream_ptr; - rt.state.input_stream.splice(ptr..ptr, res); + + // todo: this should be cheaper than splice, + // but there should be a better way to do this: + for item in res { + rt.state.input_stream.push_front(item); + } } else { tracing::warn!("tried to write to unknown file descriptor {fd}"); } From 56137fe514e678554039a560b3beab6341282fbf Mon Sep 17 00:00:00 2001 From: nhtyy Date: Thu, 12 Dec 2024 15:39:04 -0800 Subject: [PATCH 2/4] feat(hook): rsa --- crates/core/executor/src/hook.rs | 18 ++++++++++++++++++ crates/curves/src/lib.rs | 2 +- crates/zkvm/lib/src/io.rs | 2 ++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/crates/core/executor/src/hook.rs b/crates/core/executor/src/hook.rs index eb17a6ed92..287c0bdeef 100644 --- a/crates/core/executor/src/hook.rs +++ b/crates/core/executor/src/hook.rs @@ -7,6 +7,7 @@ use sp1_curves::{ ecdsa::RecoveryId as ecdsaRecoveryId, k256::{Invert, RecoveryId, Signature, VerifyingKey}, p256::{Signature as p256Signature, VerifyingKey as p256VerifyingKey}, + BigUint, Integer }; use crate::Executor; @@ -92,6 +93,7 @@ impl<'a> Default for HookRegistry<'a> { (FD_K1_ECRECOVER_HOOK, hookify(hook_k1_ecrecover)), (FD_R1_ECRECOVER_HOOK, hookify(hook_r1_ecrecover)), (FD_EDDECOMPRESS, hookify(hook_ed_decompress)), + (FD_RSA_MUL_MOD, hookify(hook_rsa_mul_mod)), ]); Self { table } @@ -231,7 +233,23 @@ pub fn hook_r1_ecrecover(_: HookEnv, buf: &[u8]) -> Vec> { #[must_use] pub fn hook_rsa_mul_mod(_: HookEnv, buf: &[u8]) -> Vec> { + assert_eq!(buf.len(), 256 + 256 + 256, "rsa_mul_mod input should have length 256 + 256 + 256, this is a bug."); + let prod: &[u8; 512] = buf[..512].try_into().unwrap(); + let m: &[u8; 256] = buf[512..].try_into().unwrap(); + + let prod = BigUint::from_bytes_le(prod); + let m = BigUint::from_bytes_le(m); + + let (q, rem) = prod.div_rem(&m); + + let mut rem = rem.to_bytes_le(); + rem.resize(512, 0); + + let mut q = q.to_bytes_le(); + q.resize(256, 0); + + vec![rem, q] } #[cfg(test)] diff --git a/crates/curves/src/lib.rs b/crates/curves/src/lib.rs index 73682a74d6..29b813c2da 100644 --- a/crates/curves/src/lib.rs +++ b/crates/curves/src/lib.rs @@ -68,7 +68,7 @@ use std::{ }; use typenum::Unsigned; -use num::BigUint; +pub use num::{Integer, BigUint}; use serde::{de::DeserializeOwned, Serialize}; pub const NUM_WORDS_FIELD_ELEMENT: usize = 8; diff --git a/crates/zkvm/lib/src/io.rs b/crates/zkvm/lib/src/io.rs index 6ab651fe05..34a06170c4 100644 --- a/crates/zkvm/lib/src/io.rs +++ b/crates/zkvm/lib/src/io.rs @@ -18,6 +18,8 @@ pub const FD_K1_ECRECOVER_HOOK: u32 = 5; pub const FD_R1_ECRECOVER_HOOK: u32 = 6; /// The file descriptor through which to access `hook_ed_decompress`. pub const FD_EDDECOMPRESS: u32 = 8; +/// The file descriptor through which to access `hook_rsa_mul_mod`. +pub const FD_RSA_MUL_MOD: u32 = 9; /// A writer that writes to a file descriptor inside the zkVM. struct SyscallWriter { From edd8a1973036c47cb81ea52a45cc548b2748ed2c Mon Sep 17 00:00:00 2001 From: nhtyy Date: Thu, 12 Dec 2024 16:14:41 -0800 Subject: [PATCH 3/4] fix: io compile --- crates/core/executor/src/hook.rs | 2 ++ crates/core/executor/src/io.rs | 6 +++--- crates/core/executor/src/syscalls/write.rs | 6 +++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/core/executor/src/hook.rs b/crates/core/executor/src/hook.rs index 287c0bdeef..0171736849 100644 --- a/crates/core/executor/src/hook.rs +++ b/crates/core/executor/src/hook.rs @@ -244,9 +244,11 @@ pub fn hook_rsa_mul_mod(_: HookEnv, buf: &[u8]) -> Vec> { let (q, rem) = prod.div_rem(&m); let mut rem = rem.to_bytes_le(); + println!("{:?}", rem); rem.resize(512, 0); let mut q = q.to_bytes_le(); + println!("{:?}", q); q.resize(256, 0); vec![rem, q] diff --git a/crates/core/executor/src/io.rs b/crates/core/executor/src/io.rs index 767697c604..67a8f79837 100644 --- a/crates/core/executor/src/io.rs +++ b/crates/core/executor/src/io.rs @@ -18,18 +18,18 @@ impl<'a> Executor<'a> { pub fn write_stdin(&mut self, input: &T) { let mut buf = Vec::new(); bincode::serialize_into(&mut buf, input).expect("serialization failed"); - self.state.input_stream.push(buf); + self.state.input_stream.push_back(buf); } /// Write a slice of bytes to the standard input stream. pub fn write_stdin_slice(&mut self, input: &[u8]) { - self.state.input_stream.push(input.to_vec()); + self.state.input_stream.push_back(input.to_vec()); } /// Write a slice of vecs to the standard input stream. pub fn write_vecs(&mut self, inputs: &[Vec]) { for input in inputs { - self.state.input_stream.push(input.clone()); + self.state.input_stream.push_back(input.clone()); } } diff --git a/crates/core/executor/src/syscalls/write.rs b/crates/core/executor/src/syscalls/write.rs index ba5caf03e5..398f665de8 100644 --- a/crates/core/executor/src/syscalls/write.rs +++ b/crates/core/executor/src/syscalls/write.rs @@ -67,9 +67,9 @@ impl Syscall for WriteSyscall { } else if let Some(mut hook) = rt.hook_registry.get(fd) { let res = hook.invoke_hook(rt.hook_env(), slice); - // todo: this should be cheaper than splice, - // but there should be a better way to do this: - for item in res { + // Write the items in reverse order to the input stream + // to preserve the expected order when reading + for item in res.into_iter().rev() { rt.state.input_stream.push_front(item); } } else { From 1b07d1cdad400f08e4d2a9d8ba0a1b6b5eb8882b Mon Sep 17 00:00:00 2001 From: nhtyy Date: Thu, 12 Dec 2024 16:54:47 -0800 Subject: [PATCH 4/4] chore(hook): comments --- crates/core/executor/src/hook.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/core/executor/src/hook.rs b/crates/core/executor/src/hook.rs index 0171736849..69d887a7c3 100644 --- a/crates/core/executor/src/hook.rs +++ b/crates/core/executor/src/hook.rs @@ -231,6 +231,16 @@ pub fn hook_r1_ecrecover(_: HookEnv, buf: &[u8]) -> Vec> { vec![vec![1], bytes.to_vec(), s_inverse.to_bytes().to_vec()] } +/// Given the product of some 256-bit numbers and the modulus, this function does a modular +/// reduction and hints back to the vm in order to constrain it. +/// +/// # Arguments +/// +/// * `env` - The environment in which the hook is invoked. +/// * `buf` - The buffer containing the product and the modulus. +/// +/// WANRING: This function is used to perform a modular reduction outside of the zkVM context. +/// These values must be constrained by the zkVM for correctness. #[must_use] pub fn hook_rsa_mul_mod(_: HookEnv, buf: &[u8]) -> Vec> { assert_eq!(buf.len(), 256 + 256 + 256, "rsa_mul_mod input should have length 256 + 256 + 256, this is a bug."); @@ -244,11 +254,9 @@ pub fn hook_rsa_mul_mod(_: HookEnv, buf: &[u8]) -> Vec> { let (q, rem) = prod.div_rem(&m); let mut rem = rem.to_bytes_le(); - println!("{:?}", rem); rem.resize(512, 0); let mut q = q.to_bytes_le(); - println!("{:?}", q); q.resize(256, 0); vec![rem, q]