diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d3fcdd449..5e61dd2dee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ #### Upcoming Changes +* feat: Fix output serialization for cairo 1 [#1645](https://github.com/lambdaclass/cairo-vm/pull/1645) + * Reverts changes added by #1630 + * Extends the serialization of Arrays added by the `print_output` flag to Spans and Dictionaries + * Now dereferences references upon serialization + * feat: Add flag to append return values to output segment when not running in proof_mode [#1646](https://github.com/lambdaclass/cairo-vm/pull/1646) * Adds the flag `append_return_values` to both the CLI and `Cairo1RunConfig` struct. * Enabling flag will add the output builtin and the necessary instructions to append the return values to the output builtin's memory segment. diff --git a/cairo1-run/src/main.rs b/cairo1-run/src/main.rs index 9dc3367e0c..e9875babb7 100644 --- a/cairo1-run/src/main.rs +++ b/cairo1-run/src/main.rs @@ -6,24 +6,27 @@ use cairo_run::Cairo1RunConfig; use cairo_vm::{ air_public_input::PublicInputError, cairo_run::EncodeTraceError, - types::errors::program_errors::ProgramError, - vm::errors::{ - memory_errors::MemoryError, runner_errors::RunnerError, trace_errors::TraceError, - vm_errors::VirtualMachineError, + types::{errors::program_errors::ProgramError, relocatable::MaybeRelocatable}, + vm::{ + errors::{ + memory_errors::MemoryError, runner_errors::RunnerError, trace_errors::TraceError, + vm_errors::VirtualMachineError, + }, + vm_core::VirtualMachine, }, Felt252, }; use clap::{Parser, ValueHint}; use itertools::Itertools; -use serialize_output::serialize_output; use std::{ io::{self, Write}, + iter::Peekable, path::PathBuf, + slice::Iter, }; use thiserror::Error; pub mod cairo_run; -pub mod serialize_output; #[derive(Parser, Debug)] #[clap(author, version, about, long_about = None)] @@ -302,7 +305,7 @@ fn main() -> Result<(), Error> { Err(Error::Cli(err)) => err.exit(), Ok(output) => { if let Some(output_string) = output { - println!("{}", output_string); + println!("Program Output : {}", output_string); } Ok(()) } @@ -328,6 +331,116 @@ fn main() -> Result<(), Error> { } } +/// Serializes the return values in a user-friendly format +/// Displays Arrays using brackets ([]) and Dictionaries using ({}) +/// Recursively dereferences referenced values (such as Span & Box) +pub fn serialize_output(vm: &VirtualMachine, return_values: &[MaybeRelocatable]) -> String { + let mut output_string = String::new(); + let mut return_values_iter: Peekable> = return_values.iter().peekable(); + serialize_output_inner(&mut return_values_iter, &mut output_string, vm); + fn serialize_output_inner( + iter: &mut Peekable>, + output_string: &mut String, + vm: &VirtualMachine, + ) { + while let Some(val) = iter.next() { + if let MaybeRelocatable::RelocatableValue(x) = val { + // Check if the next value is a relocatable of the same index + if let Some(MaybeRelocatable::RelocatableValue(y)) = iter.peek() { + // Check if the two relocatable values represent a valid array in memory + if x.segment_index == y.segment_index && x.offset <= y.offset { + // Fetch the y value from the iterator so we don't serialize it twice + iter.next(); + // Fetch array + maybe_add_whitespace(output_string); + output_string.push('['); + let array = vm.get_continuous_range(*x, y.offset - x.offset).unwrap(); + let mut array_iter: Peekable> = + array.iter().peekable(); + serialize_output_inner(&mut array_iter, output_string, vm); + output_string.push(']'); + continue; + } + } + + // Check if the single relocatable value represents a span + // In this case, the reloacatable will point us to the (start, end) pair in memory + // For example, the relocatable value may be 14:0, with the segment 14 containing [13:0 13:4] which is a valid array + if let (Ok(x), Ok(y)) = (vm.get_relocatable(*x), vm.get_relocatable(x + 1)) { + if x.segment_index == y.segment_index && y.offset >= x.offset { + // Fetch array + maybe_add_whitespace(output_string); + output_string.push('['); + let array = vm.get_continuous_range(x, y.offset - x.offset).unwrap(); + let mut array_iter: Peekable> = + array.iter().peekable(); + serialize_output_inner(&mut array_iter, output_string, vm); + output_string.push(']'); + continue; + } + } + + // Check if the relocatable value represents a dictionary + // To do so we can check if the relocatable consists of the last dict_ptr, which should be a pointer to the next empty cell in the dictionary's segment + // We can check that the dict_ptr's offset is consistent with the length of the segment and that the segment is made up of tuples of three elements (key, prev_val, val) + if x.offset + == vm + .get_segment_size(x.segment_index as usize) + .unwrap_or_default() + && x.offset % 3 == 0 + { + // Fetch the dictionary's memory + let dict_mem = vm + .get_continuous_range((x.segment_index, 0).into(), x.offset) + .expect("Malformed dictionary memory"); + // Serialize the dictionary + output_string.push('{'); + // The dictionary's memory is made up of (key, prev_value, next_value) tuples + // The prev value is not relevant to the user so we can skip over it for calrity + for (key, _, value) in dict_mem.iter().tuples() { + maybe_add_whitespace(output_string); + // Serialize the key wich should always be a Felt value + output_string.push_str(&key.to_string()); + output_string.push(':'); + // Serialize the value + // We create a peekable array here in order to use the serialize_output_inner as the value could be a span + let value_vec = vec![value.clone()]; + let mut value_iter: Peekable> = + value_vec.iter().peekable(); + serialize_output_inner(&mut value_iter, output_string, vm); + } + output_string.push('}'); + continue; + } + + // Finally, if the relocatable is neither the start of an array, a span, or a dictionary, it should be a reference (Such as Box) + // In this case we show the referenced value (if it exists) + // As this reference can also hold a reference we use the serialize_output_inner function to handle it recursively + if let Some(val) = vm.get_maybe(x) { + maybe_add_whitespace(output_string); + let array = vec![val.clone()]; + let mut array_iter: Peekable> = array.iter().peekable(); + serialize_output_inner(&mut array_iter, output_string, vm); + continue; + } + } + maybe_add_whitespace(output_string); + output_string.push_str(&val.to_string()); + } + } + + fn maybe_add_whitespace(string: &mut String) { + if !string.is_empty() + && !string.ends_with('[') + && !string.ends_with(':') + && !string.ends_with('{') + { + string.push(' '); + } + } + output_string +} + #[cfg(test)] mod tests { use super::*; @@ -531,7 +644,7 @@ mod tests { #[case(["cairo1-run", "../cairo_programs/cairo-1-programs/felt_dict.cairo", "--print_output", "--trace_file", "/dev/null", "--memory_file", "/dev/null", "--layout", "all_cairo", "--proof_mode", "--air_public_input", "/dev/null", "--air_private_input", "/dev/null"].as_slice())] fn test_run_felt_dict(#[case] args: &[&str]) { let args = args.iter().cloned().map(String::from); - let expected_output = "{\n\t0x10473: [0x8,0x9,0xa,0xb,],\n\t0x10474: [0x1,0x2,0x3,],\n}\n"; + let expected_output = "{66675:[8 9 10 11] 66676:[1 2 3]}"; assert_matches!(run(args), Ok(Some(res)) if res == expected_output); } @@ -540,7 +653,25 @@ mod tests { #[case(["cairo1-run", "../cairo_programs/cairo-1-programs/felt_span.cairo", "--print_output", "--trace_file", "/dev/null", "--memory_file", "/dev/null", "--layout", "all_cairo", "--proof_mode", "--air_public_input", "/dev/null", "--air_private_input", "/dev/null"].as_slice())] fn test_run_felt_span(#[case] args: &[&str]) { let args = args.iter().cloned().map(String::from); - let expected_output = "[0x8,0x9,0xa,0xb,]"; + let expected_output = "[8 9 10 11]"; + assert_matches!(run(args), Ok(Some(res)) if res == expected_output); + } + + #[rstest] + #[case(["cairo1-run", "../cairo_programs/cairo-1-programs/array_integer_tuple.cairo", "--print_output", "--trace_file", "/dev/null", "--memory_file", "/dev/null", "--layout", "all_cairo", "--cairo_pie_output", "/dev/null"].as_slice())] + #[case(["cairo1-run", "../cairo_programs/cairo-1-programs/array_integer_tuple.cairo", "--print_output", "--trace_file", "/dev/null", "--memory_file", "/dev/null", "--layout", "all_cairo", "--proof_mode", "--air_public_input", "/dev/null", "--air_private_input", "/dev/null"].as_slice())] + fn test_run_array_integer_tuple(#[case] args: &[&str]) { + let args = args.iter().cloned().map(String::from); + let expected_output = "[1] 1"; + assert_matches!(run(args), Ok(Some(res)) if res == expected_output); + } + + #[rstest] + #[case(["cairo1-run", "../cairo_programs/cairo-1-programs/nullable_box_vec.cairo", "--print_output", "--trace_file", "/dev/null", "--memory_file", "/dev/null", "--layout", "all_cairo", "--cairo_pie_output", "/dev/null"].as_slice())] + #[case(["cairo1-run", "../cairo_programs/cairo-1-programs/nullable_box_vec.cairo", "--print_output", "--trace_file", "/dev/null", "--memory_file", "/dev/null", "--layout", "all_cairo", "--proof_mode", "--air_public_input", "/dev/null", "--air_private_input", "/dev/null"].as_slice())] + fn test_run_nullable_box_vec(#[case] args: &[&str]) { + let args = args.iter().cloned().map(String::from); + let expected_output = "{0:10 1:20 2:30} 3"; assert_matches!(run(args), Ok(Some(res)) if res == expected_output); } } diff --git a/cairo1-run/src/serialize_output.rs b/cairo1-run/src/serialize_output.rs deleted file mode 100644 index 562f2dc832..0000000000 --- a/cairo1-run/src/serialize_output.rs +++ /dev/null @@ -1,169 +0,0 @@ -use cairo_vm::{ - types::relocatable::{MaybeRelocatable, Relocatable}, - vm::{errors::memory_errors::MemoryError, vm_core::VirtualMachine}, - Felt252, -}; -use itertools::Itertools; -use std::{collections::HashMap, iter::Peekable, slice::Iter}; -use thiserror::Error; - -#[derive(Debug)] -pub(crate) enum Output { - Felt(Felt252), - FeltSpan(Vec), - FeltDict(HashMap), -} - -#[derive(Debug, Error)] -pub struct FormatError; - -impl std::fmt::Display for FormatError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "Format error") - } -} - -impl Output { - pub fn from_memory( - vm: &VirtualMachine, - relocatable: &Relocatable, - ) -> Result { - match vm.get_relocatable(*relocatable) { - Ok(relocatable_value) => { - let segment_size = vm - .get_segment_size(relocatable_value.segment_index as usize) - .ok_or(FormatError)?; - let segment_data = vm - .get_continuous_range(relocatable_value, segment_size) - .map_err(|_| FormatError)?; - - // check if the segment data is a valid array of felts - if segment_data - .iter() - .all(|v| matches!(v, MaybeRelocatable::Int(_))) - { - let span_segment: Vec = segment_data - .iter() - .map(|v| Output::Felt(v.get_int().unwrap())) - .collect(); - Ok(Output::FeltSpan(span_segment)) - } else { - Err(FormatError) - } - } - Err(MemoryError::UnknownMemoryCell(relocatable_value)) => { - // here we assume that the value is a dictionary - let mut felt252dict: HashMap = HashMap::new(); - - let segment_size = vm - .get_segment_size(relocatable_value.segment_index as usize) - .ok_or(FormatError)?; - let mut segment_start = relocatable_value.clone(); - segment_start.offset = 0; - let segment_data = vm - .get_continuous_range(*segment_start, segment_size) - .map_err(|_| FormatError)?; - - for (dict_key, _, value_relocatable) in segment_data.iter().tuples() { - let key = dict_key.get_int().ok_or(FormatError)?; - let value_segment = value_relocatable.get_relocatable().ok_or(FormatError)?; - let value = Output::from_memory(vm, &value_segment)?; - felt252dict.insert(key, value); - } - Ok(Output::FeltDict(felt252dict)) - } - _ => Err(FormatError), - } - } -} - -impl std::fmt::Display for Output { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Output::Felt(felt) => write!(f, "{}", felt.to_hex_string()), - Output::FeltSpan(span) => { - write!(f, "[")?; - for elem in span { - write!(f, "{}", elem)?; - write!(f, ",")?; - } - write!(f, "]")?; - Ok(()) - } - Output::FeltDict(felt_dict) => { - let mut keys: Vec<_> = felt_dict.keys().collect(); - keys.sort(); - writeln!(f, "{{")?; - for key in keys { - writeln!(f, "\t{}: {},", key.to_hex_string(), felt_dict[key])?; - } - writeln!(f, "}}")?; - Ok(()) - } - } - } -} - -pub(crate) fn serialize_output(vm: &VirtualMachine, return_values: &[MaybeRelocatable]) -> String { - let mut output_string = String::new(); - let mut return_values_iter: Peekable> = return_values.iter().peekable(); - let result = serialize_output_inner(&mut return_values_iter, &mut output_string, vm); - if result.is_err() { - return result.err().unwrap().to_string(); - } - - output_string -} - -fn maybe_add_whitespace(string: &mut String) { - if !string.is_empty() && !string.ends_with('[') { - string.push(' '); - } -} - -fn serialize_output_inner( - iter: &mut Peekable>, - output_string: &mut String, - vm: &VirtualMachine, -) -> Result<(), FormatError> { - while let Some(val) = iter.next() { - match val { - MaybeRelocatable::Int(x) => { - maybe_add_whitespace(output_string); - output_string.push_str(&x.to_string()); - continue; - } - MaybeRelocatable::RelocatableValue(x) if ((iter.len() + 1) % 2) == 0 /* felt array */ => { - // Check if the next value is a relocatable of the same index - let y = iter.next().unwrap().get_relocatable().ok_or(FormatError)?; - // Check if the two relocatable values represent a valid array in memory - if x.segment_index == y.segment_index && x.offset <= y.offset { - // Fetch array - maybe_add_whitespace(output_string); - output_string.push('['); - let array = vm.get_continuous_range(*x, y.offset - x.offset).map_err(|_| FormatError)?; - let mut array_iter: Peekable> = - array.iter().peekable(); - serialize_output_inner(&mut array_iter, output_string, vm)?; - output_string.push(']'); - continue; - } - }, - MaybeRelocatable::RelocatableValue(x) if iter.len() > 1 => { - let mut segment_start = *x; - segment_start.offset = 0; - for elem in iter.into_iter() { - let output_value = Output::from_memory(vm, &elem.get_relocatable().ok_or(FormatError)?)?; - output_string.push_str(output_value.to_string().as_str()) - } - } - MaybeRelocatable::RelocatableValue(x) => { - match Output::from_memory(vm, x) { - Ok(output_value) => output_string.push_str(format!("{}", output_value).as_str()), - Err(_) => output_string.push_str("The output could not be formatted"), - } - } - } - } - Ok(()) -} diff --git a/cairo_programs/cairo-1-programs/array_integer_tuple.cairo b/cairo_programs/cairo-1-programs/array_integer_tuple.cairo new file mode 100644 index 0000000000..cb018f3a4a --- /dev/null +++ b/cairo_programs/cairo-1-programs/array_integer_tuple.cairo @@ -0,0 +1,9 @@ +use core::array::ArrayTrait; + + +fn main() -> (Array, u32) { + let mut numbers = ArrayTrait::new(); + numbers.append(1); + + (numbers, 1) +} diff --git a/cairo_programs/cairo-1-programs/nullable_box_vec.cairo b/cairo_programs/cairo-1-programs/nullable_box_vec.cairo new file mode 100644 index 0000000000..37b2b8042a --- /dev/null +++ b/cairo_programs/cairo-1-programs/nullable_box_vec.cairo @@ -0,0 +1,19 @@ +struct NullableVec { + items: Felt252Dict>>, + len: usize, +} + +fn main() -> NullableVec { + let mut d: Felt252Dict>> = Default::default(); + + // Populate the dictionary + d.insert(0, nullable_from_box(BoxTrait::new(BoxTrait::new(10)))); + d.insert(1, nullable_from_box(BoxTrait::new(BoxTrait::new(20)))); + d.insert(2, nullable_from_box(BoxTrait::new(BoxTrait::new(30)))); + + // Return NullableVec + NullableVec { + items: d, + len: 3, + } +}