Use BuiltinName enum instead of string representation

CHANGELOG.md

@@ -2,6 +2,25 @@
 
 #### Upcoming Changes
 
+* refactor(BREAKING): Use `BuiltinName` enum instead of string representation [#1722](https://github.com/lambdaclass/cairo-vm/pull/1722)
+  * `BuiltinName` moved from `crate::serde::deserialize_program` module to `crate::types::builtin_name`.
+    * Implement `BuiltinName` methods `to_str`, `to_str_with_suffix`, `from_str` & `from_str_with_suffix`.
+  * Remove `BuiltinName` method `name`.
+  * All builtin-related error variants now store `BuiltinName` instead of `&'static str` or `String`.
+  * Remove constants: `OUTPUT_BUILTIN_NAME`, `HASH_BUILTIN_NAME`, `RANGE_CHECK_BUILTIN_NAME`,`RANGE_CHECK_96_BUILTIN_NAME`, `SIGNATURE_BUILTIN_NAME`, `BITWISE_BUILTIN_NAME`, `EC_OP_BUILTIN_NAME`, `KECCAK_BUILTIN_NAME`, `POSEIDON_BUILTIN_NAME`, `SEGMENT_ARENA_BUILTIN_NAME`, `ADD_MOD_BUILTIN_NAME` &
+`MUL_MOD_BUILTIN_NAME`.
+  * Remove `BuiltinRunner` & `ModBuiltinRunner` method `identifier`
+  * Structs containing string representation of builtin names now use `BuiltinName` instead:
+    * `AirPrivateInput(pub HashMap<&'static str, Vec<PrivateInput>>)` ->  `AirPrivateInput(pub HashMap<BuiltinName, Vec<PrivateInput>>)`.
+    * `CairoPieMetadata` field `additional_data`: `HashMap<String, BuiltinAdditionalData>,` -> `CairoPieAdditionalData` with `CairoPieAdditionalData(pub HashMap<BuiltinName, BuiltinAdditionalData>)`
+    * `CairoPieMetadata` field `builtin_segments`: `HashMap<String, SegmentInfo>` -> `HashMap<BuiltinName, SegmentInfo>`.
+    * `ExecutiobResources` field `builtin_instance_counter`: `HashMap<String, usize>` -> `HashMap<BuiltinName, usize>`
+  * Methods returning string representation of builtin names now use `BuiltinName` instead:
+    * `BuiltinRunner`, `ModBuiltinRunner` & `RangeCheckBuiltinRunner` method `name`: `&'static str` -> `BuiltinName`.
+    * `CairoRunner` method `get_builtin_segment_info_for_pie`: `Result<HashMap<String, cairo_pie::SegmentInfo>, RunnerError>` -> `Result<HashMap<BuiltinName, cairo_pie::SegmentInfo>, RunnerError>`
+
+  Notes: Serialization of vm outputs that now contain `BuiltinName` & `Display` implementation of `BuiltinName` have not been affected by this PR
+
 * refactor(BREAKING): Use an enum to represent layout name[#1715](https://github.com/lambdaclass/cairo-vm/pull/1715)
   * Add enum `LayoutName` to represent cairo layout names.
   * `CairoRunConfig`, `Cairo1RunConfig` & `CairoRunner` field `layout` type changed from `String` to `LayoutName`.

cairo1-run/src/cairo_run.rs

@@ -29,19 +29,14 @@ use cairo_lang_utils::{casts::IntoOrPanic, unordered_hash_map::UnorderedHashMap}
 use cairo_vm::{
     hint_processor::cairo_1_hint_processor::hint_processor::Cairo1HintProcessor,
     math_utils::signed_felt,
-    serde::deserialize_program::{
-        ApTracking, BuiltinName, FlowTrackingData, HintParams, ReferenceManager,
+    serde::deserialize_program::{ApTracking, FlowTrackingData, HintParams, ReferenceManager},
+    types::{
+        builtin_name::BuiltinName, layout_name::LayoutName, program::Program,
+        relocatable::MaybeRelocatable,
     },
-    types::{layout_name::LayoutName, program::Program, relocatable::MaybeRelocatable},
     vm::{
         errors::{runner_errors::RunnerError, vm_errors::VirtualMachineError},
-        runners::{
-            builtin_runner::{
-                BITWISE_BUILTIN_NAME, EC_OP_BUILTIN_NAME, HASH_BUILTIN_NAME, OUTPUT_BUILTIN_NAME,
-                POSEIDON_BUILTIN_NAME, RANGE_CHECK_BUILTIN_NAME, SIGNATURE_BUILTIN_NAME,
-            },
-            cairo_runner::{CairoRunner, RunResources, RunnerMode},
-        },
+        runners::cairo_runner::{CairoRunner, RunResources, RunnerMode},
         vm_core::VirtualMachine,
     },
     Felt252,
@@ -236,10 +231,7 @@ pub fn cairo_run_program(
                     .iter()
                     .enumerate()
                     .map(|(i, builtin)| {
-                        (
-                            builtin.name(),
-                            (vm.get_ap() - (builtins.len() - 1 - i)).unwrap(),
-                        )
+                        (*builtin, (vm.get_ap() - (builtins.len() - 1 - i)).unwrap())
                     })
                     .collect(),
                 false,
@@ -723,14 +715,14 @@ fn finalize_builtins(
     let mut builtin_name_to_stack_pointer = HashMap::new();
     for (id, size) in ret_types_and_sizes {
         if let Some(ref name) = id.debug_name {
-            let builtin_name = match &*name.to_string() {
-                "RangeCheck" => RANGE_CHECK_BUILTIN_NAME,
-                "Poseidon" => POSEIDON_BUILTIN_NAME,
-                "EcOp" => EC_OP_BUILTIN_NAME,
-                "Bitwise" => BITWISE_BUILTIN_NAME,
-                "Pedersen" => HASH_BUILTIN_NAME,
-                "Output" => OUTPUT_BUILTIN_NAME,
-                "Ecdsa" => SIGNATURE_BUILTIN_NAME,
+            let builtin_name = match name.as_str() {
+                "RangeCheck" => BuiltinName::range_check,
+                "Poseidon" => BuiltinName::poseidon,
+                "EcOp" => BuiltinName::ec_op,
+                "Bitwise" => BuiltinName::bitwise,
+                "Pedersen" => BuiltinName::pedersen,
+                "Output" => BuiltinName::output,
+                "Ecdsa" => BuiltinName::ecdsa,
                 _ => {
                     stack_pointer.offset += size as usize;
                     continue;

vm/src/air_private_input.rs

@@ -3,11 +3,7 @@ use crate::{
         collections::{BTreeMap, HashMap},
         prelude::{String, Vec},
     },
-    vm::runners::builtin_runner::{
-        ADD_MOD_BUILTIN_NAME, BITWISE_BUILTIN_NAME, EC_OP_BUILTIN_NAME, HASH_BUILTIN_NAME,
-        KECCAK_BUILTIN_NAME, MUL_MOD_BUILTIN_NAME, POSEIDON_BUILTIN_NAME, RANGE_CHECK_BUILTIN_NAME,
-        SIGNATURE_BUILTIN_NAME,
-    },
+    types::builtin_name::BuiltinName,
 };
 use serde::{Deserialize, Serialize};
 
@@ -40,7 +36,7 @@ pub struct AirPrivateInputSerializable {
 
 // Contains only builtin public inputs, useful for library users
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub struct AirPrivateInput(pub HashMap<&'static str, Vec<PrivateInput>>);
+pub struct AirPrivateInput(pub HashMap<BuiltinName, Vec<PrivateInput>>);
 
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
 #[serde(untagged)]
@@ -159,21 +155,21 @@ impl AirPrivateInput {
         AirPrivateInputSerializable {
             trace_path,
             memory_path,
-            pedersen: self.0.get(HASH_BUILTIN_NAME).cloned(),
-            range_check: self.0.get(RANGE_CHECK_BUILTIN_NAME).cloned(),
-            ecdsa: self.0.get(SIGNATURE_BUILTIN_NAME).cloned(),
-            bitwise: self.0.get(BITWISE_BUILTIN_NAME).cloned(),
-            ec_op: self.0.get(EC_OP_BUILTIN_NAME).cloned(),
-            keccak: self.0.get(KECCAK_BUILTIN_NAME).cloned(),
-            poseidon: self.0.get(POSEIDON_BUILTIN_NAME).cloned(),
+            pedersen: self.0.get(&BuiltinName::pedersen).cloned(),
+            range_check: self.0.get(&BuiltinName::range_check).cloned(),
+            ecdsa: self.0.get(&BuiltinName::ecdsa).cloned(),
+            bitwise: self.0.get(&BuiltinName::bitwise).cloned(),
+            ec_op: self.0.get(&BuiltinName::ec_op).cloned(),
+            keccak: self.0.get(&BuiltinName::keccak).cloned(),
+            poseidon: self.0.get(&BuiltinName::poseidon).cloned(),
             add_mod: self
                 .0
-                .get(ADD_MOD_BUILTIN_NAME)
+                .get(&BuiltinName::add_mod)
                 .and_then(|pi| pi.first())
                 .cloned(),
             mul_mod: self
                 .0
-                .get(MUL_MOD_BUILTIN_NAME)
+                .get(&BuiltinName::mul_mod)
                 .and_then(|pi| pi.first())
                 .cloned(),
         }
@@ -188,13 +184,13 @@ impl From<AirPrivateInputSerializable> for AirPrivateInput {
                 inputs.insert(input_name, input);
             }
         };
-        insert_input(HASH_BUILTIN_NAME, private_input.pedersen);
-        insert_input(RANGE_CHECK_BUILTIN_NAME, private_input.range_check);
-        insert_input(SIGNATURE_BUILTIN_NAME, private_input.ecdsa);
-        insert_input(BITWISE_BUILTIN_NAME, private_input.bitwise);
-        insert_input(EC_OP_BUILTIN_NAME, private_input.ec_op);
-        insert_input(KECCAK_BUILTIN_NAME, private_input.keccak);
-        insert_input(POSEIDON_BUILTIN_NAME, private_input.poseidon);
+        insert_input(BuiltinName::pedersen, private_input.pedersen);
+        insert_input(BuiltinName::range_check, private_input.range_check);
+        insert_input(BuiltinName::ecdsa, private_input.ecdsa);
+        insert_input(BuiltinName::bitwise, private_input.bitwise);
+        insert_input(BuiltinName::ec_op, private_input.ec_op);
+        insert_input(BuiltinName::keccak, private_input.keccak);
+        insert_input(BuiltinName::poseidon, private_input.poseidon);
 
         Self(inputs)
     }
@@ -213,10 +209,6 @@ mod tests {
     use {
         super::*,
         crate::air_private_input::{AirPrivateInput, AirPrivateInputSerializable},
-        crate::vm::runners::builtin_runner::{
-            BITWISE_BUILTIN_NAME, EC_OP_BUILTIN_NAME, HASH_BUILTIN_NAME, KECCAK_BUILTIN_NAME,
-            POSEIDON_BUILTIN_NAME, RANGE_CHECK_BUILTIN_NAME, SIGNATURE_BUILTIN_NAME,
-        },
         assert_matches::assert_matches,
     };
 
@@ -285,13 +277,13 @@ mod tests {
 
         let private_input = AirPrivateInput::from(serializable_private_input.clone());
 
-        assert_matches!(private_input.0.get(HASH_BUILTIN_NAME), data if data == serializable_private_input.pedersen.as_ref());
-        assert_matches!(private_input.0.get(RANGE_CHECK_BUILTIN_NAME), data if data == serializable_private_input.range_check.as_ref());
-        assert_matches!(private_input.0.get(SIGNATURE_BUILTIN_NAME), data if data == serializable_private_input.ecdsa.as_ref());
-        assert_matches!(private_input.0.get(BITWISE_BUILTIN_NAME), data if data == serializable_private_input.bitwise.as_ref());
-        assert_matches!(private_input.0.get(EC_OP_BUILTIN_NAME), data if data == serializable_private_input.ec_op.as_ref());
-        assert_matches!(private_input.0.get(KECCAK_BUILTIN_NAME), data if data == serializable_private_input.keccak.as_ref());
-        assert_matches!(private_input.0.get(POSEIDON_BUILTIN_NAME), data if data == serializable_private_input.poseidon.as_ref());
+        assert_matches!(private_input.0.get(&BuiltinName::pedersen), data if data == serializable_private_input.pedersen.as_ref());
+        assert_matches!(private_input.0.get(&BuiltinName::range_check), data if data == serializable_private_input.range_check.as_ref());
+        assert_matches!(private_input.0.get(&BuiltinName::ecdsa), data if data == serializable_private_input.ecdsa.as_ref());
+        assert_matches!(private_input.0.get(&BuiltinName::bitwise), data if data == serializable_private_input.bitwise.as_ref());
+        assert_matches!(private_input.0.get(&BuiltinName::ec_op), data if data == serializable_private_input.ec_op.as_ref());
+        assert_matches!(private_input.0.get(&BuiltinName::keccak), data if data == serializable_private_input.keccak.as_ref());
+        assert_matches!(private_input.0.get(&BuiltinName::poseidon), data if data == serializable_private_input.poseidon.as_ref());
     }
 
     #[test]

vm/src/program_hash.rs

@@ -2,8 +2,8 @@ use starknet_crypto::{pedersen_hash, FieldElement};
 
 use crate::Felt252;
 
-use crate::serde::deserialize_program::BuiltinName;
 use crate::stdlib::vec::Vec;
+use crate::types::builtin_name::BuiltinName;
 use crate::types::relocatable::MaybeRelocatable;
 use crate::vm::runners::cairo_pie::StrippedProgram;
 
@@ -58,15 +58,12 @@ where
 ///
 /// Converts the builtin name to bytes then attempts to create a field element from
 /// these bytes. This function will fail if the builtin name is over 31 characters.
-fn builtin_to_field_element(builtin: &BuiltinName) -> Result<FieldElement, ProgramHashError> {
+fn builtin_name_to_field_element(
+    builtin_name: &BuiltinName,
+) -> Result<FieldElement, ProgramHashError> {
     // The Python implementation uses the builtin name without suffix
-    let builtin_name = builtin
-        .name()
-        .strip_suffix("_builtin")
-        .unwrap_or(builtin.name());
-
-    FieldElement::from_byte_slice_be(builtin_name.as_bytes())
-        .map_err(|_| ProgramHashError::InvalidProgramBuiltin(builtin.name()))
+    FieldElement::from_byte_slice_be(builtin_name.to_str().as_bytes())
+        .map_err(|_| ProgramHashError::InvalidProgramBuiltin(builtin_name.to_str()))
 }
 
 /// The `value: FieldElement` is `pub(crate)` and there is no accessor.
@@ -111,7 +108,7 @@ pub fn compute_program_hash_chain(
     let builtin_list: Result<Vec<FieldElement>, _> = program
         .builtins
         .iter()
-        .map(builtin_to_field_element)
+        .map(builtin_name_to_field_element)
         .collect();
     let builtin_list = builtin_list?;
 

vm/src/serde/deserialize_program.rs

@@ -1,7 +1,7 @@
 //! # Program deserialization
 //!
 //! This module contains the logic for [`Program`] deserialization.
-//! Users shouldn't need to use it directly (except for [`BuiltinName`]).
+//! Users shouldn't need to use it directly
 //!
 //! To generate a [`Program`] from a JSON string, see [`Program::from_bytes()`].
 //! To do the same from a JSON file, see [`Program::from_file()`].
@@ -13,13 +13,11 @@ use crate::{
         prelude::*,
         sync::Arc,
     },
+    types::builtin_name::BuiltinName,
     utils::CAIRO_PRIME,
-    vm::runners::builtin_runner::RANGE_CHECK_96_BUILTIN_NAME,
 };
 
 use crate::utils::PRIME_STR;
-use crate::vm::runners::builtin_runner::SEGMENT_ARENA_BUILTIN_NAME;
-use crate::vm::runners::builtin_runner::{ADD_MOD_BUILTIN_NAME, MUL_MOD_BUILTIN_NAME};
 use crate::Felt252;
 use crate::{
     serde::deserialize_utils,
@@ -29,11 +27,6 @@ use crate::{
         program::{HintsCollection, Program, SharedProgramData},
         relocatable::MaybeRelocatable,
     },
-    vm::runners::builtin_runner::{
-        BITWISE_BUILTIN_NAME, EC_OP_BUILTIN_NAME, HASH_BUILTIN_NAME, KECCAK_BUILTIN_NAME,
-        OUTPUT_BUILTIN_NAME, POSEIDON_BUILTIN_NAME, RANGE_CHECK_BUILTIN_NAME,
-        SIGNATURE_BUILTIN_NAME,
-    },
 };
 use num_bigint::BigUint;
 use num_traits::{float::FloatCore, Num};
@@ -43,44 +36,6 @@ use serde_json::Number;
 #[cfg(all(feature = "arbitrary", feature = "std"))]
 use arbitrary::{self, Arbitrary, Unstructured};
 
-// This enum is used to deserialize program builtins into &str and catch non-valid names
-#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
-#[derive(Serialize, Deserialize, Debug, PartialEq, Copy, Clone, Eq, Hash)]
-#[allow(non_camel_case_types)]
-pub enum BuiltinName {
-    output,
-    range_check,
-    pedersen,
-    ecdsa,
-    keccak,
-    bitwise,
-    ec_op,
-    poseidon,
-    segment_arena,
-    range_check96,
-    add_mod,
-    mul_mod,
-}
-
-impl BuiltinName {
-    pub fn name(&self) -> &'static str {
-        match self {
-            BuiltinName::output => OUTPUT_BUILTIN_NAME,
-            BuiltinName::range_check => RANGE_CHECK_BUILTIN_NAME,
-            BuiltinName::pedersen => HASH_BUILTIN_NAME,
-            BuiltinName::ecdsa => SIGNATURE_BUILTIN_NAME,
-            BuiltinName::keccak => KECCAK_BUILTIN_NAME,
-            BuiltinName::bitwise => BITWISE_BUILTIN_NAME,
-            BuiltinName::ec_op => EC_OP_BUILTIN_NAME,
-            BuiltinName::poseidon => POSEIDON_BUILTIN_NAME,
-            BuiltinName::segment_arena => SEGMENT_ARENA_BUILTIN_NAME,
-            BuiltinName::range_check96 => RANGE_CHECK_96_BUILTIN_NAME,
-            BuiltinName::add_mod => ADD_MOD_BUILTIN_NAME,
-            BuiltinName::mul_mod => MUL_MOD_BUILTIN_NAME,
-        }
-    }
-}
-
 #[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary, Clone))]
 #[derive(Deserialize, Debug)]
 pub struct ProgramJson {

vm/src/serde/serialize_program.rs

@@ -1,13 +1,16 @@
-use crate::stdlib::{
-    collections::{BTreeMap, HashMap},
-    prelude::*,
+use crate::{
+    stdlib::{
+        collections::{BTreeMap, HashMap},
+        prelude::*,
+    },
+    types::builtin_name::BuiltinName,
 };
 
 use serde::{Deserialize, Serialize};
 
 use super::deserialize_program::{
-    ApTracking, Attribute, BuiltinName, DebugInfo, FlowTrackingData, HintParams, Identifier,
-    Member, ProgramJson, Reference, ReferenceManager, ValueAddress,
+    ApTracking, Attribute, DebugInfo, FlowTrackingData, HintParams, Identifier, Member,
+    ProgramJson, Reference, ReferenceManager, ValueAddress,
 };
 use crate::types::program::Program;
 use crate::types::relocatable::MaybeRelocatable;