Use_btreemap_for_deterministic_order_of_pie_keys (#2085)

Author: YairVaknin-starkware

vm/src/tests/cairo_pie_test.rs

@@ -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::{
@@ -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))),
@@ -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 {
@@ -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)));
@@ -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)),
@@ -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

vm/src/types/builtin_name.rs

@@ -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,
@@ -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
@@ -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"))
     }
 }

vm/src/vm/runners/cairo_pie.rs

@@ -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,
 };
@@ -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)]
@@ -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>,
 }
 
@@ -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;
 
@@ -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

vm/src/vm/runners/cairo_runner.rs

@@ -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(),
         })
     }
 
@@ -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,
         };
 
@@ -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
@@ -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(),
             }),
         );
     }
@@ -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(),
             }),
         );
     }
@@ -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)]),
             }),
         );
     }
@@ -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 {
@@ -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),
             ]),
@@ -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)
                 ])
@@ -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()
             }
         );
     }