Merge pull request #1010 from 0xPolygonMiden/frisitano-stack-outputs-…

…constructor

Stack outputs constructor error handling

CHANGELOG.md

@@ -8,6 +8,7 @@
 #### VM Internals
 - Simplified range checker and removed 1 main and 1 auxiliary trace column (#949).
 - Added `get_mapped_values()` and `get_store_subset()` methods to the `AdviceProvider` trait (#987).
+- Improved handling of invalid/incomplete parameters in `StackOutputs` constructors (#1010).
 
 ## 0.6.1 (2023-06-29)
 

core/src/errors.rs

@@ -25,3 +25,32 @@ impl fmt::Display for InputError {
 
 #[cfg(feature = "std")]
 impl std::error::Error for InputError {}
+
+// OUTPUT ERROR
+// ================================================================================================
+#[derive(Clone, Debug)]
+pub enum OutputError {
+    InvalidOverflowAddress(u64),
+    InvalidOverflowAddressLength(usize, usize),
+    InvalidStackElement(u64),
+}
+
+impl fmt::Display for OutputError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        use OutputError::*;
+        match self {
+            InvalidOverflowAddress(address) => {
+                write!(f, "overflow addresses contains {address} that is not a valid field element")
+            }
+            InvalidOverflowAddressLength(actual, expected) => {
+                write!(f, "overflow addresses length is {actual}, but expected {expected}")
+            }
+            InvalidStackElement(element) => {
+                write!(f, "stack contains {element} that is not a valid field element")
+            }
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for OutputError {}

core/src/stack/mod.rs

@@ -1,4 +1,7 @@
-use super::{errors::InputError, Felt, StackTopState, StarkField, ToElements};
+use super::{
+    errors::{InputError, OutputError},
+    Felt, StackTopState, StarkField, ToElements,
+};
 use winter_utils::{
     collections::{vec, Vec},
     ByteWriter, Serializable,

core/src/stack/outputs.rs

@@ -1,5 +1,6 @@
 use super::{
-    ByteWriter, Felt, Serializable, StackTopState, StarkField, ToElements, Vec, STACK_TOP_SIZE,
+    ByteWriter, Felt, OutputError, Serializable, StackTopState, StarkField, ToElements, Vec,
+    STACK_TOP_SIZE,
 };
 
 // STACK OUTPUTS
@@ -30,30 +31,54 @@ pub struct StackOutputs {
 impl StackOutputs {
     // CONSTRUCTOR
     // --------------------------------------------------------------------------------------------
-    pub fn new(stack: Vec<u64>, overflow_addrs: Vec<u64>) -> Self {
-        debug_assert!(
-            are_valid_elements(&stack),
-            "stack outputs contain values that are not valid field elements",
-        );
-        debug_assert!(
-            are_valid_elements(&overflow_addrs),
-            "overflow address outputs contain values that are not valid field elements",
-        );
-
-        Self {
+    /// Constructs a new [StackOutputs] struct from the provided stack elements and overflow
+    /// addresses.
+    ///
+    /// # Errors
+    /// - If any of the provided stack elements are invalid field elements.
+    /// - If any of the provided overflow addresses are invalid field elements.
+    /// - If the number of stack elements is greater than `STACK_TOP_SIZE` (16) and `overflow_addrs`
+    ///   does not contain exactly `stack.len() + 1 - STACK_TOP_SIZE` elements.
+    pub fn new(mut stack: Vec<u64>, overflow_addrs: Vec<u64>) -> Result<Self, OutputError> {
+        // Validate stack elements
+        if let Some(element) = find_invalid_elements(&stack) {
+            return Err(OutputError::InvalidStackElement(element));
+        }
+
+        // Validate overflow address elements
+        if let Some(element) = find_invalid_elements(&overflow_addrs) {
+            return Err(OutputError::InvalidOverflowAddress(element));
+        }
+
+        // pad stack to the `STACK_TOP_SIZE`
+        if stack.len() < STACK_TOP_SIZE {
+            stack.resize(STACK_TOP_SIZE, 0);
+        }
+
+        // validate overflow_addrs length
+        let expected_overflow_addrs_len = if stack.len() > STACK_TOP_SIZE {
+            stack.len() + 1 - STACK_TOP_SIZE
+        } else {
+            0
+        };
+        if overflow_addrs.len() != expected_overflow_addrs_len {
+            return Err(OutputError::InvalidOverflowAddressLength(
+                overflow_addrs.len(),
+                expected_overflow_addrs_len,
+            ));
+        }
+
+        Ok(Self {
             stack,
             overflow_addrs,
-        }
+        })
     }
 
-    pub fn from_elements(stack: Vec<Felt>, overflow_addrs: Vec<Felt>) -> Self {
+    pub fn from_elements(stack: Vec<Felt>, overflow_addrs: Vec<Felt>) -> Result<Self, OutputError> {
         let stack = stack.iter().map(|&v| v.as_int()).collect::<Vec<_>>();
         let overflow_addrs = overflow_addrs.iter().map(|&v| v.as_int()).collect::<Vec<_>>();
 
-        Self {
-            stack,
-            overflow_addrs,
-        }
+        Self::new(stack, overflow_addrs)
     }
 
     // PUBLIC ACCESSORS
@@ -129,14 +154,14 @@ impl StackOutputs {
 // HELPER FUNCTIONS
 // ================================================================================================
 
-/// Verify that each element in the provided slice of outputs is a valid field element.
-fn are_valid_elements(outputs: &[u64]) -> bool {
+/// Find and return the first invalid field element in the provided vector of elements.
+fn find_invalid_elements(outputs: &[u64]) -> Option<u64> {
     for val in outputs {
         if *val >= Felt::MODULUS {
-            return false;
+            return Some(*val);
         }
     }
-    true
+    None
 }
 
 impl Serializable for StackOutputs {

miden/src/cli/data.rs

@@ -316,7 +316,7 @@ impl OutputFile {
     }
 
     /// Converts outputs vectors for stack and overflow addresses to [StackOutputs].
-    pub fn stack_outputs(&self) -> StackOutputs {
+    pub fn stack_outputs(&self) -> Result<StackOutputs, String> {
         let stack = self.stack.iter().map(|v| v.parse::<u64>().unwrap()).collect::<Vec<u64>>();
 
         let overflow_addrs = self
@@ -326,6 +326,7 @@ impl OutputFile {
             .collect::<Vec<u64>>();
 
         StackOutputs::new(stack, overflow_addrs)
+            .map_err(|e| format!("Construct stack outputs failed {e}"))
     }
 }
 

miden/src/cli/verify.rs

@@ -49,7 +49,7 @@ impl VerifyCmd {
         let program_info = ProgramInfo::new(program_hash, kernel);
 
         // verify proof
-        verifier::verify(program_info, stack_inputs, outputs_data.stack_outputs(), proof)
+        verifier::verify(program_info, stack_inputs, outputs_data.stack_outputs()?, proof)
             .map_err(|err| format!("Program failed verification! - {}", err))?;
 
         println!("Verification complete in {} ms", now.elapsed().as_millis());

processor/src/stack/mod.rs

@@ -141,6 +141,7 @@ impl Stack {
         self.trace.append_state_into(&mut stack_items, self.clk);
         self.overflow.append_into(&mut stack_items);
         StackOutputs::from_elements(stack_items, self.overflow.get_addrs())
+            .expect("processor stack handling logic is valid")
     }
 
     // TRACE ACCESSORS AND MUTATORS