Skip to content

Use_btreemap_for_deterministic_order_of_pie_keys #2085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 5, 2025
Merged

Use_btreemap_for_deterministic_order_of_pie_keys #2085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions vm/src/tests/cairo_pie_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use wasm_bindgen_test::*;
use crate::{
cairo_run::{cairo_run, CairoRunConfig},
hint_processor::builtin_hint_processor::builtin_hint_processor_definition::BuiltinHintProcessor,
stdlib::collections::HashMap,
stdlib::collections::{BTreeMap, HashMap},
types::relocatable::Relocatable,
vm::runners::{
cairo_pie::{
Expand Down Expand Up @@ -45,7 +45,7 @@ fn pedersen_test() {
// ret_pc_segment
assert_eq!(pie_metadata.ret_pc_segment, SegmentInfo::from((6, 0)));
// builtin_segments
let expected_builtin_segments = HashMap::from([
let expected_builtin_segments = BTreeMap::from([
(BuiltinName::output, SegmentInfo::from((2, 1))),
(BuiltinName::pedersen, SegmentInfo::from((3, 3))),
(BuiltinName::range_check, SegmentInfo::from((4, 0))),
Expand All @@ -71,15 +71,15 @@ fn pedersen_test() {
let expected_execution_resources = ExecutionResources {
n_steps: 14,
n_memory_holes: 0,
builtin_instance_counter: HashMap::from([
builtin_instance_counter: BTreeMap::from([
(BuiltinName::range_check, 0),
(BuiltinName::output, 1),
(BuiltinName::pedersen, 1),
]),
};
assert_eq!(cairo_pie.execution_resources, expected_execution_resources);
// additional_data
let expected_additional_data = HashMap::from([
let expected_additional_data = BTreeMap::from([
(
BuiltinName::output,
BuiltinAdditionalData::Output(OutputBuiltinAdditionalData {
Expand Down Expand Up @@ -128,7 +128,7 @@ fn common_signature() {
assert_eq!(pie_metadata.ret_pc_segment, SegmentInfo::from((4, 0)));
// builtin_segments
let expected_builtin_segments =
HashMap::from([(BuiltinName::ecdsa, SegmentInfo::from((2, 2)))]);
BTreeMap::from([(BuiltinName::ecdsa, SegmentInfo::from((2, 2)))]);
assert_eq!(pie_metadata.builtin_segments, expected_builtin_segments);
// program_segment
assert_eq!(pie_metadata.program_segment, SegmentInfo::from((0, 21)));
Expand All @@ -150,11 +150,11 @@ fn common_signature() {
let expected_execution_resources = ExecutionResources {
n_steps: 11,
n_memory_holes: 0,
builtin_instance_counter: HashMap::from([(BuiltinName::ecdsa, 1)]),
builtin_instance_counter: BTreeMap::from([(BuiltinName::ecdsa, 1)]),
};
assert_eq!(cairo_pie.execution_resources, expected_execution_resources);
// additional_data
let expected_additional_data = HashMap::from([(
let expected_additional_data = BTreeMap::from([(
BuiltinName::ecdsa,
BuiltinAdditionalData::Signature(HashMap::from([(
Relocatable::from((2, 0)),
Expand Down Expand Up @@ -223,7 +223,7 @@ fn relocate_segments() {
let expected_execution_resources = ExecutionResources {
n_steps: 22,
n_memory_holes: 0,
builtin_instance_counter: HashMap::default(),
builtin_instance_counter: BTreeMap::default(),
};
assert_eq!(cairo_pie.execution_resources, expected_execution_resources);
// additional_data
Expand Down
12 changes: 6 additions & 6 deletions vm/src/types/builtin_name.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const MUL_MOD_BUILTIN_NAME_WITH_SUFFIX: &str = "mul_mod_builtin";

/// Enum representing the name of a cairo builtin
#[cfg_attr(feature = "test_utils", derive(Arbitrary))]
#[derive(Serialize, Deserialize, Debug, PartialEq, Copy, Clone, Eq, Hash)]
#[derive(Serialize, Deserialize, Debug, PartialEq, Copy, Clone, Eq, Hash, Ord, PartialOrd)]
#[allow(non_camel_case_types)]
pub enum BuiltinName {
output,
Expand Down Expand Up @@ -179,11 +179,11 @@ impl core::fmt::Display for BuiltinName {
// Implementation of custom serialization & deserialization for maps using builtin names with suffixes as keys
pub(crate) mod serde_generic_map_impl {
use super::BuiltinName;
use crate::stdlib::{collections::HashMap, string::String};
use crate::stdlib::{collections::BTreeMap, string::String};
use serde::{de::Error, ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer};

pub fn serialize<S, V>(
values: &HashMap<BuiltinName, V>,
values: &BTreeMap<BuiltinName, V>,
serializer: S,
) -> Result<S::Ok, S::Error>
where
Expand All @@ -199,13 +199,13 @@ pub(crate) mod serde_generic_map_impl {

pub fn deserialize<'de, D: Deserializer<'de>, V: Deserialize<'de>>(
d: D,
) -> Result<HashMap<BuiltinName, V>, D::Error> {
) -> Result<BTreeMap<BuiltinName, V>, D::Error> {
// First deserialize keys into String
let map = HashMap::<String, V>::deserialize(d)?;
let map = BTreeMap::<String, V>::deserialize(d)?;
// Then match keys to BuiltinName and handle invalid names
map.into_iter()
.map(|(k, v)| BuiltinName::from_str_with_suffix(&k).map(|k| (k, v)))
.collect::<Option<HashMap<_, _>>>()
.collect::<Option<BTreeMap<_, _>>>()
.ok_or(D::Error::custom("Invalid builtin name"))
}
}
13 changes: 8 additions & 5 deletions vm/src/vm/runners/cairo_pie.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ use crate::stdlib::prelude::{String, Vec};
use crate::types::builtin_name::BuiltinName;
use crate::vm::errors::cairo_pie_errors::CairoPieValidationError;
use crate::{
stdlib::{collections::HashMap, prelude::*},
stdlib::{
collections::{BTreeMap, HashMap},
prelude::*,
},
types::relocatable::{MaybeRelocatable, Relocatable},
Felt252,
};
Expand Down Expand Up @@ -125,7 +128,7 @@ impl PartialEq for BuiltinAdditionalData {
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
pub struct CairoPieAdditionalData(
#[serde(with = "crate::types::builtin_name::serde_generic_map_impl")]
pub HashMap<BuiltinName, BuiltinAdditionalData>,
pub BTreeMap<BuiltinName, BuiltinAdditionalData>,
);

#[derive(Serialize, Clone, Debug, PartialEq, Eq)]
Expand All @@ -145,7 +148,7 @@ pub struct CairoPieMetadata {
pub ret_fp_segment: SegmentInfo,
pub ret_pc_segment: SegmentInfo,
#[serde(serialize_with = "serde_impl::serialize_builtin_segments")]
pub builtin_segments: HashMap<BuiltinName, SegmentInfo>,
pub builtin_segments: BTreeMap<BuiltinName, SegmentInfo>,
pub extra_segments: Vec<SegmentInfo>,
}

Expand Down Expand Up @@ -435,7 +438,7 @@ impl CairoPie {
}

pub(super) mod serde_impl {
use crate::stdlib::collections::HashMap;
use crate::stdlib::collections::{BTreeMap, HashMap};
use crate::types::builtin_name::BuiltinName;
use num_traits::Num;

Expand Down Expand Up @@ -840,7 +843,7 @@ pub(super) mod serde_impl {
}

pub fn serialize_builtin_segments<S>(
values: &HashMap<BuiltinName, SegmentInfo>,
values: &BTreeMap<BuiltinName, SegmentInfo>,
serializer: S,
) -> Result<S::Ok, S::Error>
where
Expand Down
26 changes: 13 additions & 13 deletions vm/src/vm/runners/cairo_runner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,7 @@ impl CairoRunner {
Ok(ExecutionResources {
n_steps,
n_memory_holes,
builtin_instance_counter,
builtin_instance_counter: builtin_instance_counter.into_iter().collect(),
})
}

Expand Down Expand Up @@ -1415,7 +1415,7 @@ impl CairoRunner {
execution_segment: (execution_base.segment_index, execution_size).into(),
ret_fp_segment: (return_fp.segment_index, 0).into(),
ret_pc_segment: (return_pc.segment_index, 0).into(),
builtin_segments,
builtin_segments: builtin_segments.into_iter().collect(),
extra_segments,
};

Expand Down Expand Up @@ -1597,7 +1597,7 @@ pub struct ExecutionResources {
pub n_steps: usize,
pub n_memory_holes: usize,
#[serde(with = "crate::types::builtin_name::serde_generic_map_impl")]
pub builtin_instance_counter: HashMap<BuiltinName, usize>,
pub builtin_instance_counter: BTreeMap<BuiltinName, usize>,
}

/// Returns a copy of the execution resources where all the builtins with a usage counter
Expand Down Expand Up @@ -3965,7 +3965,7 @@ mod tests {
Ok(ExecutionResources {
n_steps: 10,
n_memory_holes: 0,
builtin_instance_counter: HashMap::new(),
builtin_instance_counter: BTreeMap::new(),
}),
);
}
Expand Down Expand Up @@ -4020,7 +4020,7 @@ mod tests {
Ok(ExecutionResources {
n_steps: 10,
n_memory_holes: 0,
builtin_instance_counter: HashMap::new(),
builtin_instance_counter: BTreeMap::new(),
}),
);
}
Expand All @@ -4045,7 +4045,7 @@ mod tests {
Ok(ExecutionResources {
n_steps: 10,
n_memory_holes: 0,
builtin_instance_counter: HashMap::from([(BuiltinName::output, 4)]),
builtin_instance_counter: BTreeMap::from([(BuiltinName::output, 4)]),
}),
);
}
Expand Down Expand Up @@ -4989,7 +4989,7 @@ mod tests {
}

fn setup_execution_resources() -> (ExecutionResources, ExecutionResources) {
let mut builtin_instance_counter: HashMap<BuiltinName, usize> = HashMap::new();
let mut builtin_instance_counter: BTreeMap<BuiltinName, usize> = BTreeMap::new();
builtin_instance_counter.insert(BuiltinName::output, 8);

let execution_resources_1 = ExecutionResources {
Expand Down Expand Up @@ -5181,7 +5181,7 @@ mod tests {
let execution_resources_1 = ExecutionResources {
n_steps: 800,
n_memory_holes: 0,
builtin_instance_counter: HashMap::from([
builtin_instance_counter: BTreeMap::from([
(BuiltinName::pedersen, 7),
(BuiltinName::range_check, 16),
]),
Expand All @@ -5192,7 +5192,7 @@ mod tests {
ExecutionResources {
n_steps: 1600,
n_memory_holes: 0,
builtin_instance_counter: HashMap::from([
builtin_instance_counter: BTreeMap::from([
(BuiltinName::pedersen, 14),
(BuiltinName::range_check, 32)
])
Expand All @@ -5202,30 +5202,30 @@ mod tests {
let execution_resources_2 = ExecutionResources {
n_steps: 545,
n_memory_holes: 0,
builtin_instance_counter: HashMap::from([(BuiltinName::range_check, 17)]),
builtin_instance_counter: BTreeMap::from([(BuiltinName::range_check, 17)]),
};

assert_eq!(
&execution_resources_2 * 8,
ExecutionResources {
n_steps: 4360,
n_memory_holes: 0,
builtin_instance_counter: HashMap::from([(BuiltinName::range_check, 136)])
builtin_instance_counter: BTreeMap::from([(BuiltinName::range_check, 136)])
}
);

let execution_resources_3 = ExecutionResources {
n_steps: 42,
n_memory_holes: 0,
builtin_instance_counter: HashMap::new(),
builtin_instance_counter: BTreeMap::new(),
};

assert_eq!(
&execution_resources_3 * 18,
ExecutionResources {
n_steps: 756,
n_memory_holes: 0,
builtin_instance_counter: HashMap::new()
builtin_instance_counter: BTreeMap::new()
}
);
}
Expand Down
Loading