From d18a56110e1e4e075b78f240d06bd299b29c98b2 Mon Sep 17 00:00:00 2001 From: Ben Goebel Date: Tue, 19 Sep 2023 16:55:29 -0600 Subject: [PATCH 1/6] update serialize memory for bin format --- vm/src/vm/runners/cairo_pie.rs | 138 +++++++++++---------------------- 1 file changed, 45 insertions(+), 93 deletions(-) diff --git a/vm/src/vm/runners/cairo_pie.rs b/vm/src/vm/runners/cairo_pie.rs index 54890a28a3..364953d48f 100644 --- a/vm/src/vm/runners/cairo_pie.rs +++ b/vm/src/vm/runners/cairo_pie.rs @@ -102,24 +102,23 @@ mod serde_impl { types::relocatable::{MaybeRelocatable, Relocatable}, utils::CAIRO_PRIME, }; - #[cfg(any(target_arch = "wasm32", no_std, not(feature = "std")))] - use alloc::collections::{btree_map::Entry, BTreeMap}; use felt::Felt252; + use num_bigint::BigUint; + use num_traits::Num; use serde::{ ser::{SerializeSeq, SerializeTuple}, Serialize, Serializer, }; - #[cfg(not(any(target_arch = "wasm32", no_std, not(feature = "std"))))] - use std::collections::{btree_map::Entry, BTreeMap}; + + const ADDR_BYTE_LEN: usize = 8; + const FIELD_BYTE_LEN: usize = 32; + const ADDR_BASE: usize = 0x8000000000000000; // 2 ** (8 * ADDR_BYTE_LEN - 1) + const OFFSET_BASE: usize = 0x800000000000; // 2 ** OFFSET_BIT_LEN + const RELOCATE_BASE: &str = "8000000000000000000000000000000000000000000000000000000000000000"; // 2 ** (8 * FIELD_BYTE_LEN - 1) struct Felt252Wrapper<'a>(&'a Felt252); struct RelocatableWrapper<'a>(&'a Relocatable); - struct MissingSegment; - - struct MemoryData<'a>(&'a BTreeMap); - struct MemoryHole; - impl<'a> Serialize for Felt252Wrapper<'a> { fn serialize(&self, serializer: S) -> Result where @@ -147,61 +146,6 @@ mod serde_impl { } } - impl Serialize for MissingSegment { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - serializer.serialize_none() - } - } - - impl<'a> Serialize for MemoryData<'a> { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - let mut seq_serializer = serializer.serialize_seq(Some( - self.0 - .last_key_value() - .map(|x| *x.0 + 1) - .unwrap_or_default(), - ))?; - - let mut last_offset = None; - for (offset, value) in self.0.iter() { - // Serialize memory holes as `None`. - for _ in last_offset.map(|x| x + 1).unwrap_or_default()..*offset { - seq_serializer.serialize_element(&MemoryHole)?; - } - - // Update the last offset to check for memory holes after itself. - last_offset = Some(*offset); - - // Serialize the data. - match value { - MaybeRelocatable::RelocatableValue(x) => { - seq_serializer.serialize_element(&RelocatableWrapper(x))? - } - MaybeRelocatable::Int(x) => { - seq_serializer.serialize_element(&Felt252Wrapper(x))? - } - } - } - - seq_serializer.end() - } - } - - impl Serialize for MemoryHole { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - serializer.serialize_none() - } - } - pub fn serialize_program_data( values: &[MaybeRelocatable], serializer: S, @@ -230,38 +174,46 @@ mod serde_impl { where S: Serializer, { - let mut memory = BTreeMap::new(); - for value in values { - let segment_entry = match memory.entry(value.0 .0) { - Entry::Vacant(x) => x.insert(BTreeMap::new()), - Entry::Occupied(x) => x.into_mut(), - }; + // TODO: update current test, add new test + // Missing segment and memory holes can be ignored + // as they can be inferred by the address on the prover side + let mem_cap = values.len() * ADDR_BYTE_LEN + values.len() * FIELD_BYTE_LEN; + let mut res = Vec::with_capacity(mem_cap); - segment_entry.insert(value.0 .1, &value.1); - } - - let mut seq_serializer = serializer.serialize_seq(Some( - memory - .last_entry() - .map(|x| *x.key() + 1) - .unwrap_or_default(), - ))?; - - let mut last_segment = None; - for (segment_idx, segment_data) in memory { - // Serialize missing segments as `None`. - for _ in last_segment.map(|x| x + 1).unwrap_or_default()..segment_idx { - seq_serializer.serialize_element(&MissingSegment)?; - } - - // Update the last segment to check for missing segments after itself. - last_segment = Some(segment_idx); - - // Serialize the data. - seq_serializer.serialize_element(&MemoryData(&segment_data))?; + for ((segment, offset), value) in values.iter() { + match value { + // Serializes RelocatableValue as(little endian): + // 1bit | SEGMENT_BITS | OFFSET_BITS + // 1 | segment | offset + MaybeRelocatable::RelocatableValue(rel_val) => { + let mem_addr = ADDR_BASE + *segment * OFFSET_BASE + *offset; + + let reloc_base = BigUint::from_str_radix(RELOCATE_BASE, 16) + .map_err(|_| serde::ser::Error::custom("invalid int str"))?; + let reloc_value = reloc_base + + BigUint::from(rel_val.segment_index as usize) + * BigUint::from(OFFSET_BASE) + + BigUint::from(rel_val.offset); + res.extend_from_slice(mem_addr.to_le_bytes().as_ref()); + res.extend_from_slice(reloc_value.to_bytes_le().as_ref()); + } + // Serializes Int as(little endian): + // 1bit | Num + // 0 | num + MaybeRelocatable::Int(data_val) => { + let mem_addr = ADDR_BASE + *segment * OFFSET_BASE + *offset; + res.extend_from_slice(mem_addr.to_le_bytes().as_ref()); + res.extend_from_slice(data_val.to_le_bytes().as_ref()); + } + }; } - seq_serializer.end() + serializer.serialize_str( + res.iter() + .map(|b| format!("{:x}", b)) + .collect::() + .as_str(), + ) } pub fn serialize_prime(_value: &(), serializer: S) -> Result From 4cff44b551df8723daa5a65e77cf46131c704f1b Mon Sep 17 00:00:00 2001 From: Ben Goebel Date: Wed, 20 Sep 2023 17:06:04 -0600 Subject: [PATCH 2/6] integration and unit tests --- CHANGELOG.md | 2 + vm/src/tests/cairo_pie_test_output.json | 49 +------------ vm/src/vm/runners/cairo_pie.rs | 92 ++++++++++++++++++------- 3 files changed, 70 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 661e7627e1..5d19320b09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ #### Upcoming Changes +* fix: Change serialization of CairoPieMemory to match Python's binary format [#1447](https://github.com/lambdaclass/cairo-vm/pull/1447) + * fix: Remove Deserialize derive from CairoPie and fix Serialize implementation to match Python's [#1444](https://github.com/lambdaclass/cairo-vm/pull/1444) * fix: ec_recover hints no longer panic when divisor is 0 [#1433](https://github.com/lambdaclass/cairo-vm/pull/1433) diff --git a/vm/src/tests/cairo_pie_test_output.json b/vm/src/tests/cairo_pie_test_output.json index 3d83f71382..4d5bc55837 100644 --- a/vm/src/tests/cairo_pie_test_output.json +++ b/vm/src/tests/cairo_pie_test_output.json @@ -12,54 +12,7 @@ }, "n_steps": 7 }, - "memory": [ - [ - 4612671182993129469, - 5198983563776393216, - 1, - 2345108766317314046, - 5191102247248822272, - 5189976364521848832, - 1234, - 1226245742482522112, - 3618502788666131213697322783095070105623107215331596699973092056135872020474, - 2345108766317314046 - ], - [ - [ - 2, - 0 - ], - [ - 3, - 0 - ], - [ - 4, - 0 - ], - [ - 2, - 0 - ], - 1234, - [ - 1, - 3 - ], - [ - 0, - 9 - ], - [ - 2, - 1 - ] - ], - [ - 1234 - ] - ], + "memory": "0000000000000080fd7ffc7f0080034000000000000000000000000000000000000000000000000001000000000000800080fc7f01802648000000000000000000000000000000000000000000000000020000000000008001000000000000000000000000000000000000000000000000000000000000000300000000000080fe7fff7fff7f8b2000000000000000000000000000000000000000000000000004000000000000800080ff7ffd7f0a4800000000000000000000000000000000000000000000000005000000000000800080ff7f018006480000000000000000000000000000000000000000000000000600000000000080d204000000000000000000000000000000000000000000000000000000000000070000000000008000800180018004110000000000000000000000000000000000000000000000000800000000000080faffffffffffffffffffffffffffffffffffffffffffffff10000000000000080900000000000080fe7fff7fff7f8b20000000000000000000000000000000000000000000000000000000000080008000000000000001000000000000000000000000000000000000000000000000800100000000800080000000000080010000000000000000000000000000000000000000000000008002000000008000800000000000000200000000000000000000000000000000000000000000000080030000000080008000000000000001000000000000000000000000000000000000000000000000800400000000800080d2040000000000000000000000000000000000000000000000000000000000000500000000800080030000000080000000000000000000000000000000000000000000000000008006000000008000800900000000000000000000000000000000000000000000000000000000000080070000000080008001000000000001000000000000000000000000000000000000000000000000800000000000000180d204000000000000000000000000000000000000000000000000000000000000", "metadata": { "extra_segments": [], "ret_pc_segment": { diff --git a/vm/src/vm/runners/cairo_pie.rs b/vm/src/vm/runners/cairo_pie.rs index 364953d48f..9d4118f119 100644 --- a/vm/src/vm/runners/cairo_pie.rs +++ b/vm/src/vm/runners/cairo_pie.rs @@ -110,11 +110,12 @@ mod serde_impl { Serialize, Serializer, }; - const ADDR_BYTE_LEN: usize = 8; - const FIELD_BYTE_LEN: usize = 32; - const ADDR_BASE: usize = 0x8000000000000000; // 2 ** (8 * ADDR_BYTE_LEN - 1) - const OFFSET_BASE: usize = 0x800000000000; // 2 ** OFFSET_BIT_LEN - const RELOCATE_BASE: &str = "8000000000000000000000000000000000000000000000000000000000000000"; // 2 ** (8 * FIELD_BYTE_LEN - 1) + pub const ADDR_BYTE_LEN: usize = 8; + pub const FIELD_BYTE_LEN: usize = 32; + pub const ADDR_BASE: usize = 0x8000000000000000; // 2 ** (8 * ADDR_BYTE_LEN - 1) + pub const OFFSET_BASE: usize = 0x800000000000; // 2 ** OFFSET_BIT_LEN + pub const RELOCATE_BASE: &str = + "8000000000000000000000000000000000000000000000000000000000000000"; // 2 ** (8 * FIELD_BYTE_LEN - 1) struct Felt252Wrapper<'a>(&'a Felt252); struct RelocatableWrapper<'a>(&'a Relocatable); @@ -174,7 +175,6 @@ mod serde_impl { where S: Serializer, { - // TODO: update current test, add new test // Missing segment and memory holes can be ignored // as they can be inferred by the address on the prover side let mem_cap = values.len() * ADDR_BYTE_LEN + values.len() * FIELD_BYTE_LEN; @@ -182,14 +182,14 @@ mod serde_impl { for ((segment, offset), value) in values.iter() { match value { - // Serializes RelocatableValue as(little endian): + // Serializes RelocatableValue(little endian): // 1bit | SEGMENT_BITS | OFFSET_BITS // 1 | segment | offset MaybeRelocatable::RelocatableValue(rel_val) => { let mem_addr = ADDR_BASE + *segment * OFFSET_BASE + *offset; let reloc_base = BigUint::from_str_radix(RELOCATE_BASE, 16) - .map_err(|_| serde::ser::Error::custom("invalid int str"))?; + .map_err(|_| serde::ser::Error::custom("invalid relocation base str"))?; let reloc_value = reloc_base + BigUint::from(rel_val.segment_index as usize) * BigUint::from(OFFSET_BASE) @@ -197,7 +197,7 @@ mod serde_impl { res.extend_from_slice(mem_addr.to_le_bytes().as_ref()); res.extend_from_slice(reloc_value.to_bytes_le().as_ref()); } - // Serializes Int as(little endian): + // Serializes Int(little endian): // 1bit | Num // 0 | num MaybeRelocatable::Int(data_val) => { @@ -208,9 +208,10 @@ mod serde_impl { }; } + println!("MEM: {:?}", res); serializer.serialize_str( res.iter() - .map(|b| format!("{:x}", b)) + .map(|b| format!("{:02x}", b)) .collect::() .as_str(), ) @@ -238,7 +239,6 @@ mod serde_impl { #[cfg(test)] mod test { use super::*; - use serde_json::json; #[test] fn serialize_cairo_pie_memory() { @@ -247,24 +247,66 @@ mod test { #[serde(serialize_with = "serde_impl::serialize_memory")] CairoPieMemory, ); + let addrs = [ + ((1, 0), "0000000000800080"), + ((1, 1), "0100000000800080"), + ((1, 4), "0400000000800080"), + ((1, 8), "0800000000800080"), + ((2, 0), "0000000000000180"), + ((5, 8), "0800000000800280"), + ]; + let memory = MemoryWrapper(vec![ - ((1, 0), MaybeRelocatable::Int(10.into())), - ((1, 1), MaybeRelocatable::Int(11.into())), - ((1, 4), MaybeRelocatable::Int(12.into())), - ((1, 8), MaybeRelocatable::RelocatableValue((1, 2).into())), - ((2, 0), MaybeRelocatable::RelocatableValue((3, 4).into())), - ((4, 8), MaybeRelocatable::RelocatableValue((5, 6).into())), + (addrs[0].0, MaybeRelocatable::Int(1234.into())), + (addrs[1].0, MaybeRelocatable::Int(11.into())), + (addrs[2].0, MaybeRelocatable::Int(12.into())), + ( + addrs[3].0, + MaybeRelocatable::RelocatableValue((1, 2).into()), + ), + ( + addrs[4].0, + MaybeRelocatable::RelocatableValue((3, 4).into()), + ), + ( + addrs[5].0, + MaybeRelocatable::RelocatableValue((5, 6).into()), + ), ]); + let mem = serde_json::to_value(memory).unwrap(); + let mem_str = mem.as_str().unwrap(); + let shift_len = (serde_impl::ADDR_BYTE_LEN + serde_impl::FIELD_BYTE_LEN) * 2; + let shift_field = serde_impl::FIELD_BYTE_LEN * 2; + let shift_addr = serde_impl::ADDR_BYTE_LEN * 2; + + // Serializes Address 8 Byte(little endian): + for (i, expected_addr) in addrs.into_iter().enumerate() { + let shift = shift_len * i; + assert_eq!( + &mem_str[shift..shift + shift_addr], + expected_addr.1, + "addr mismatch({i}): {mem_str:?}" + ); + } + + // assert_eq!(mem_bytes.as_slice(), &addr_be[..]); + // Serializes Int(little endian): + // 1bit | Num + // 0 | num + assert_eq!( + &mem_str[shift_addr..shift_addr + shift_field], + "d204000000000000000000000000000000000000000000000000000000000000", + "value mismatch: {mem_str:?}" + ); + // Serializes RelocatableValue(little endian): + // 1bit | SEGMENT_BITS | OFFSET_BITS + // 1 | segment | offset + let shift_first_relocatable = shift_len * 3 + shift_addr; assert_eq!( - serde_json::to_value(memory).unwrap(), - json!([ - (), - [10, 11, (), (), 12, (), (), (), [1, 2]], - [[3, 4,]], - (), - [(), (), (), (), (), (), (), (), [5, 6]] - ]), + &mem_str[shift_first_relocatable..shift_first_relocatable + shift_field], + "0200000000800000000000000000000000000000000000000000000000000080", + "value mismatch: {mem_str:?}" ); } } From 0a40ae427b474b6661a1c81bc8996ef8058031a2 Mon Sep 17 00:00:00 2001 From: Ben Goebel Date: Wed, 20 Sep 2023 17:08:40 -0600 Subject: [PATCH 3/6] remove debug log --- vm/src/vm/runners/cairo_pie.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/vm/src/vm/runners/cairo_pie.rs b/vm/src/vm/runners/cairo_pie.rs index 9d4118f119..4e57c99e81 100644 --- a/vm/src/vm/runners/cairo_pie.rs +++ b/vm/src/vm/runners/cairo_pie.rs @@ -208,7 +208,6 @@ mod serde_impl { }; } - println!("MEM: {:?}", res); serializer.serialize_str( res.iter() .map(|b| format!("{:02x}", b)) From 78fd09582b1f6eac7fbab6c62e251b7a35e1e342 Mon Sep 17 00:00:00 2001 From: Ben Goebel Date: Mon, 25 Sep 2023 10:07:20 -0600 Subject: [PATCH 4/6] change usize -> u64, alloc for wasm --- vm/src/vm/runners/cairo_pie.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/vm/src/vm/runners/cairo_pie.rs b/vm/src/vm/runners/cairo_pie.rs index 4e57c99e81..5a7c8184c0 100644 --- a/vm/src/vm/runners/cairo_pie.rs +++ b/vm/src/vm/runners/cairo_pie.rs @@ -112,8 +112,8 @@ mod serde_impl { pub const ADDR_BYTE_LEN: usize = 8; pub const FIELD_BYTE_LEN: usize = 32; - pub const ADDR_BASE: usize = 0x8000000000000000; // 2 ** (8 * ADDR_BYTE_LEN - 1) - pub const OFFSET_BASE: usize = 0x800000000000; // 2 ** OFFSET_BIT_LEN + pub const ADDR_BASE: u64 = 0x8000000000000000; // 2 ** (8 * ADDR_BYTE_LEN - 1) + pub const OFFSET_BASE: u64 = 0x800000000000; // 2 ** OFFSET_BIT_LEN pub const RELOCATE_BASE: &str = "8000000000000000000000000000000000000000000000000000000000000000"; // 2 ** (8 * FIELD_BYTE_LEN - 1) @@ -175,6 +175,11 @@ mod serde_impl { where S: Serializer, { + #[cfg(any(target_arch = "wasm32", no_std, not(feature = "std")))] + use alloc::string::String; + #[cfg(any(target_arch = "wasm32", no_std, not(feature = "std")))] + use alloc::vec::Vec; + // Missing segment and memory holes can be ignored // as they can be inferred by the address on the prover side let mem_cap = values.len() * ADDR_BYTE_LEN + values.len() * FIELD_BYTE_LEN; @@ -186,7 +191,7 @@ mod serde_impl { // 1bit | SEGMENT_BITS | OFFSET_BITS // 1 | segment | offset MaybeRelocatable::RelocatableValue(rel_val) => { - let mem_addr = ADDR_BASE + *segment * OFFSET_BASE + *offset; + let mem_addr = ADDR_BASE + *segment as u64 * OFFSET_BASE + *offset as u64; let reloc_base = BigUint::from_str_radix(RELOCATE_BASE, 16) .map_err(|_| serde::ser::Error::custom("invalid relocation base str"))?; @@ -201,7 +206,7 @@ mod serde_impl { // 1bit | Num // 0 | num MaybeRelocatable::Int(data_val) => { - let mem_addr = ADDR_BASE + *segment * OFFSET_BASE + *offset; + let mem_addr = ADDR_BASE + *segment as u64 * OFFSET_BASE + *offset as u64; res.extend_from_slice(mem_addr.to_le_bytes().as_ref()); res.extend_from_slice(data_val.to_le_bytes().as_ref()); } @@ -289,7 +294,6 @@ mod test { ); } - // assert_eq!(mem_bytes.as_slice(), &addr_be[..]); // Serializes Int(little endian): // 1bit | Num // 0 | num From 15b25800222c0d9144dd075bfd11684c374a8d1a Mon Sep 17 00:00:00 2001 From: Ben Goebel Date: Tue, 26 Sep 2023 11:23:42 -0600 Subject: [PATCH 5/6] remove unused wrapper --- vm/src/vm/runners/cairo_pie.rs | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/vm/src/vm/runners/cairo_pie.rs b/vm/src/vm/runners/cairo_pie.rs index 5a7c8184c0..5917adae48 100644 --- a/vm/src/vm/runners/cairo_pie.rs +++ b/vm/src/vm/runners/cairo_pie.rs @@ -98,17 +98,11 @@ pub struct CairoPieVersion { mod serde_impl { use super::CAIRO_PIE_VERSION; - use crate::{ - types::relocatable::{MaybeRelocatable, Relocatable}, - utils::CAIRO_PRIME, - }; + use crate::{types::relocatable::MaybeRelocatable, utils::CAIRO_PRIME}; use felt::Felt252; use num_bigint::BigUint; use num_traits::Num; - use serde::{ - ser::{SerializeSeq, SerializeTuple}, - Serialize, Serializer, - }; + use serde::{ser::SerializeSeq, Serialize, Serializer}; pub const ADDR_BYTE_LEN: usize = 8; pub const FIELD_BYTE_LEN: usize = 32; @@ -118,7 +112,6 @@ mod serde_impl { "8000000000000000000000000000000000000000000000000000000000000000"; // 2 ** (8 * FIELD_BYTE_LEN - 1) struct Felt252Wrapper<'a>(&'a Felt252); - struct RelocatableWrapper<'a>(&'a Relocatable); impl<'a> Serialize for Felt252Wrapper<'a> { fn serialize(&self, serializer: S) -> Result @@ -133,20 +126,6 @@ mod serde_impl { } } - impl<'a> Serialize for RelocatableWrapper<'a> { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - let mut tuple_serializer = serializer.serialize_tuple(2)?; - - tuple_serializer.serialize_element(&self.0.segment_index)?; - tuple_serializer.serialize_element(&self.0.offset)?; - - tuple_serializer.end() - } - } - pub fn serialize_program_data( values: &[MaybeRelocatable], serializer: S, From 7a08b6deb6d3d16baa7d0182633ce3e551ce39ad Mon Sep 17 00:00:00 2001 From: Ben Goebel Date: Tue, 26 Sep 2023 11:28:24 -0600 Subject: [PATCH 6/6] codecov in uncovered cairopie test --- vm/src/vm/runners/cairo_pie.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vm/src/vm/runners/cairo_pie.rs b/vm/src/vm/runners/cairo_pie.rs index 5917adae48..60a0fa147e 100644 --- a/vm/src/vm/runners/cairo_pie.rs +++ b/vm/src/vm/runners/cairo_pie.rs @@ -269,7 +269,7 @@ mod test { assert_eq!( &mem_str[shift..shift + shift_addr], expected_addr.1, - "addr mismatch({i}): {mem_str:?}" + "addr mismatch({i}): {mem_str:?}", ); } @@ -279,7 +279,7 @@ mod test { assert_eq!( &mem_str[shift_addr..shift_addr + shift_field], "d204000000000000000000000000000000000000000000000000000000000000", - "value mismatch: {mem_str:?}" + "value mismatch: {mem_str:?}", ); // Serializes RelocatableValue(little endian): // 1bit | SEGMENT_BITS | OFFSET_BITS @@ -288,7 +288,7 @@ mod test { assert_eq!( &mem_str[shift_first_relocatable..shift_first_relocatable + shift_field], "0200000000800000000000000000000000000000000000000000000000000080", - "value mismatch: {mem_str:?}" + "value mismatch: {mem_str:?}", ); } }