Skip to content

Commit

Permalink
refactor: Remove unused code & use constants whenever possible for bu…
Browse files Browse the repository at this point in the history
…iltin instance definitions (#1707)

* Reduce BitwiseInstanceDef

* Cleanup module

* Fixes

* Yeet CpuInstanceDef

* Update tests

* Simplify ec op instance def

* Clean Ecdsa instance def

* Clean Keccak instance def

* Update tests

* Clean Pedersen instance def

* Remove instances_per_component field

* Use the constants instead of storing a value for n_input cells and cells_per_instance for all builtins

* Extract memory_units_per_step into a constant

* Remove underscore from used field

* Remove unused field _cpu_component_step

* Remove unused field _n_trace_columns

* Impl Default for PoseidonInstanceDef

* Fix test

* Fix test

* fmt

* Add changelog entry

* Fix field name
  • Loading branch information
fmoletta authored Apr 12, 2024
1 parent 635fef9 commit 1d23afb
Show file tree
Hide file tree
Showing 43 changed files with 402 additions and 1,004 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#### Upcoming Changes

* refactor: Remove unused code & use constants whenever possible for builtin instance definitions[#1707](https://github.com/lambdaclass/cairo-vm/pull/1707)

* fix(BREAKING): Use program builtins in `initialize_main_entrypoint` & `read_return_values`[#1703](https://github.com/lambdaclass/cairo-vm/pull/1703)
* `initialize_main_entrypoint` now iterates over the program builtins when building the stack & inserts 0 for any missing builtin
* `read_return_values` now only computes the final stack of the builtins in the program
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,6 @@ mod tests {
hint_processor_definition::HintProcessorLogic,
},
relocatable,
types::exec_scope::ExecutionScopes,
utils::test_utils::*,
vm::errors::memory_errors::MemoryError,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ mod tests {
},
hint_processor_definition::{HintProcessorLogic, HintReference},
},
types::{exec_scope::ExecutionScopes, relocatable::Relocatable},
types::relocatable::Relocatable,
utils::test_utils::*,
vm::vm_core::VirtualMachine,
};
Expand Down
1 change: 0 additions & 1 deletion vm/src/hint_processor/builtin_hint_processor/ec_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ mod tests {
use crate::hint_processor::builtin_hint_processor::builtin_hint_processor_definition::HintProcessorData;
use crate::hint_processor::hint_processor_definition::HintProcessorLogic;
use crate::relocatable;
use crate::types::exec_scope::ExecutionScopes;
use crate::types::relocatable::Relocatable;
use num_traits::Zero;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,6 @@ mod tests {
},
hint_processor_definition::HintProcessorLogic,
},
types::exec_scope::ExecutionScopes,
utils::test_utils::*,
vm::vm_core::VirtualMachine,
};
Expand Down
1 change: 0 additions & 1 deletion vm/src/hint_processor/builtin_hint_processor/garaga.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ mod tests {
use crate::hint_processor::builtin_hint_processor::builtin_hint_processor_definition::BuiltinHintProcessor;
use crate::hint_processor::builtin_hint_processor::builtin_hint_processor_definition::HintProcessorData;
use crate::hint_processor::hint_processor_definition::HintProcessorLogic;
use crate::types::exec_scope::ExecutionScopes;
use crate::Felt252;
use crate::{hint_processor::builtin_hint_processor::hint_code, utils::test_utils::*};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,6 @@ mod tests {
},
hint_processor_definition::{HintProcessorLogic, HintReference},
},
types::exec_scope::ExecutionScopes,
utils::test_utils::*,
vm::vm_core::VirtualMachine,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ mod tests {
use crate::hint_processor::builtin_hint_processor::builtin_hint_processor_definition::HintProcessorData;
use crate::hint_processor::hint_processor_definition::HintProcessorLogic;
use crate::hint_processor::hint_processor_definition::HintReference;
use crate::types::exec_scope::ExecutionScopes;
use crate::vm::vm_core::VirtualMachine;

use crate::{hint_processor::builtin_hint_processor::hint_code, utils::test_utils::*};
Expand Down
2 changes: 1 addition & 1 deletion vm/src/hint_processor/builtin_hint_processor/pow_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ mod tests {
builtin_hint_processor::builtin_hint_processor_definition::HintProcessorData,
hint_processor_definition::HintProcessorLogic,
},
types::{exec_scope::ExecutionScopes, relocatable::MaybeRelocatable},
types::relocatable::MaybeRelocatable,
utils::test_utils::*,
vm::{errors::memory_errors::MemoryError, vm_core::VirtualMachine},
};
Expand Down
1 change: 0 additions & 1 deletion vm/src/hint_processor/builtin_hint_processor/segments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ mod tests {
},
hint_processor_definition::HintProcessorLogic,
},
types::exec_scope::ExecutionScopes,
utils::test_utils::*,
vm::vm_core::VirtualMachine,
};
Expand Down
2 changes: 1 addition & 1 deletion vm/src/hint_processor/builtin_hint_processor/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ mod tests {
},
hint_processor_definition::HintProcessorLogic,
},
types::{exec_scope::ExecutionScopes, relocatable::MaybeRelocatable},
types::relocatable::MaybeRelocatable,
utils::test_utils::*,
vm::vm_core::VirtualMachine,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,6 @@ mod tests {
},
hint_processor_definition::{HintProcessorLogic, HintReference},
},
types::exec_scope::ExecutionScopes,
utils::test_utils::*,
vm::vm_core::VirtualMachine,
};
Expand Down
12 changes: 3 additions & 9 deletions vm/src/hint_processor/builtin_hint_processor/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ mod tests {
},
hint_processor_definition::HintProcessorLogic,
},
types::{
exec_scope::ExecutionScopes, instance_definitions::ecdsa_instance_def::EcdsaInstanceDef,
},
utils::test_utils::*,
vm::runners::builtin_runner::SignatureBuiltinRunner,
};
Expand All @@ -67,8 +64,7 @@ mod tests {
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
fn verify_ecdsa_signature_valid() {
let mut vm = vm!();
vm.builtin_runners =
vec![SignatureBuiltinRunner::new(&EcdsaInstanceDef::default(), true).into()];
vm.builtin_runners = vec![SignatureBuiltinRunner::new(Some(512), true).into()];
vm.segments = segments![
((1, 0), (0, 0)),
(
Expand All @@ -94,8 +90,7 @@ mod tests {
#[test]
fn verify_ecdsa_signature_invalid_ecdsa_ptr() {
let mut vm = vm!();
vm.builtin_runners =
vec![SignatureBuiltinRunner::new(&EcdsaInstanceDef::default(), true).into()];
vm.builtin_runners = vec![SignatureBuiltinRunner::new(Some(512), true).into()];
vm.segments = segments![
((1, 0), (3, 0)),
(
Expand All @@ -121,8 +116,7 @@ mod tests {
#[test]
fn verify_ecdsa_signature_invalid_input_cell() {
let mut vm = vm!();
vm.builtin_runners =
vec![SignatureBuiltinRunner::new(&EcdsaInstanceDef::default(), true).into()];
vm.builtin_runners = vec![SignatureBuiltinRunner::new(Some(512), true).into()];
vm.segments = segments![
((1, 0), (0, 3)),
(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -507,10 +507,7 @@ mod tests {
},
hint_processor_definition::HintProcessorLogic,
},
types::{
exec_scope::ExecutionScopes,
relocatable::{MaybeRelocatable, Relocatable},
},
types::relocatable::{MaybeRelocatable, Relocatable},
utils::test_utils::*,
vm::{errors::memory_errors::MemoryError, vm_core::VirtualMachine},
};
Expand Down
5 changes: 1 addition & 4 deletions vm/src/hint_processor/builtin_hint_processor/uint384.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,7 @@ mod tests {
},
hint_processor_definition::HintProcessorLogic,
},
types::{
exec_scope::ExecutionScopes,
relocatable::{MaybeRelocatable, Relocatable},
},
types::relocatable::{MaybeRelocatable, Relocatable},
utils::test_utils::*,
vm::{errors::memory_errors::MemoryError, vm_core::VirtualMachine},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ mod tests {
use crate::hint_processor::builtin_hint_processor::hint_code;
use crate::hint_processor::builtin_hint_processor::secp::bigint_utils::Uint768;
use crate::hint_processor::hint_processor_definition::HintProcessorLogic;
use crate::types::exec_scope::ExecutionScopes;
use crate::utils::test_utils::*;

use assert_matches::assert_matches;
Expand Down
1 change: 0 additions & 1 deletion vm/src/hint_processor/builtin_hint_processor/vrf/fq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ mod tests {
use crate::hint_processor::builtin_hint_processor::hint_code;
use crate::hint_processor::hint_processor_definition::HintProcessorLogic;
use crate::types::errors::math_errors::MathError;
use crate::types::exec_scope::ExecutionScopes;
use crate::utils::test_utils::*;
use assert_matches::assert_matches;

Expand Down
50 changes: 9 additions & 41 deletions vm/src/types/instance_definitions/bitwise_instance_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,22 @@ use serde::Serialize;

pub(crate) const CELLS_PER_BITWISE: u32 = 5;
pub(crate) const INPUT_CELLS_PER_BITWISE: u32 = 2;
pub(crate) const TOTAL_N_BITS: u32 = 251;

#[derive(Serialize, Clone, Debug, PartialEq)]
pub(crate) struct BitwiseInstanceDef {
pub(crate) ratio: Option<u32>,
pub(crate) total_n_bits: u32,
}

impl BitwiseInstanceDef {
pub(crate) fn default() -> Self {
BitwiseInstanceDef {
ratio: Some(256),
total_n_bits: 251,
}
impl Default for BitwiseInstanceDef {
fn default() -> Self {
BitwiseInstanceDef { ratio: Some(256) }
}
}

impl BitwiseInstanceDef {
pub(crate) fn new(ratio: Option<u32>) -> Self {
BitwiseInstanceDef {
ratio,
total_n_bits: 251,
}
}

pub(crate) fn _cells_per_builtin(&self) -> u32 {
CELLS_PER_BITWISE
}

pub(crate) fn _range_check_units_per_builtin(&self) -> u32 {
0
BitwiseInstanceDef { ratio }
}
}

Expand All @@ -40,37 +28,17 @@ mod tests {
#[cfg(target_arch = "wasm32")]
use wasm_bindgen_test::*;

#[test]
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
fn get_range_check_units_per_builtin() {
let builtin_instance = BitwiseInstanceDef::default();
assert_eq!(builtin_instance._range_check_units_per_builtin(), 0);
}

#[test]
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
fn get_cells_per_builtin() {
let builtin_instance = BitwiseInstanceDef::default();
assert_eq!(builtin_instance._cells_per_builtin(), 5);
}

#[test]
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
fn test_new() {
let builtin_instance = BitwiseInstanceDef {
ratio: Some(8),
total_n_bits: 251,
};
let builtin_instance = BitwiseInstanceDef { ratio: Some(8) };
assert_eq!(BitwiseInstanceDef::new(Some(8)), builtin_instance);
}

#[test]
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
fn test_default() {
let builtin_instance = BitwiseInstanceDef {
ratio: Some(256),
total_n_bits: 251,
};
let builtin_instance = BitwiseInstanceDef { ratio: Some(256) };
assert_eq!(BitwiseInstanceDef::default(), builtin_instance);
}
}
18 changes: 10 additions & 8 deletions vm/src/types/instance_definitions/builtins_instance_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use super::{
range_check_instance_def::RangeCheckInstanceDef,
};

pub(crate) const BUILTIN_INSTANCES_PER_COMPONENT: u32 = 1;

use serde::Serialize;

#[derive(Serialize, Debug, PartialEq)]
Expand Down Expand Up @@ -75,7 +77,7 @@ impl BuiltinsInstanceDef {
pub(crate) fn recursive() -> BuiltinsInstanceDef {
BuiltinsInstanceDef {
output: true,
pedersen: Some(PedersenInstanceDef::new(Some(128), 1)),
pedersen: Some(PedersenInstanceDef::new(Some(128))),
range_check: Some(RangeCheckInstanceDef::default()),
ecdsa: None,
bitwise: Some(BitwiseInstanceDef::new(Some(8))),
Expand All @@ -91,7 +93,7 @@ impl BuiltinsInstanceDef {
pub(crate) fn starknet() -> BuiltinsInstanceDef {
BuiltinsInstanceDef {
output: true,
pedersen: Some(PedersenInstanceDef::new(Some(32), 1)),
pedersen: Some(PedersenInstanceDef::new(Some(32))),
range_check: Some(RangeCheckInstanceDef::new(Some(16))),
ecdsa: Some(EcdsaInstanceDef::new(Some(2048))),
bitwise: Some(BitwiseInstanceDef::new(Some(64))),
Expand All @@ -107,12 +109,12 @@ impl BuiltinsInstanceDef {
pub(crate) fn starknet_with_keccak() -> BuiltinsInstanceDef {
BuiltinsInstanceDef {
output: true,
pedersen: Some(PedersenInstanceDef::new(Some(32), 1)),
pedersen: Some(PedersenInstanceDef::new(Some(32))),
range_check: Some(RangeCheckInstanceDef::new(Some(16))),
ecdsa: Some(EcdsaInstanceDef::new(Some(2048))),
bitwise: Some(BitwiseInstanceDef::new(Some(64))),
ec_op: Some(EcOpInstanceDef::new(Some(1024))),
keccak: Some(KeccakInstanceDef::new(Some(2048), vec![200; 8])),
keccak: Some(KeccakInstanceDef::new(Some(2048))),
poseidon: Some(PoseidonInstanceDef::default()),
range_check96: None,
add_mod: None,
Expand All @@ -123,7 +125,7 @@ impl BuiltinsInstanceDef {
pub(crate) fn recursive_large_output() -> BuiltinsInstanceDef {
BuiltinsInstanceDef {
output: true,
pedersen: Some(PedersenInstanceDef::new(Some(128), 1)),
pedersen: Some(PedersenInstanceDef::new(Some(128))),
range_check: Some(RangeCheckInstanceDef::default()),
ecdsa: None,
bitwise: Some(BitwiseInstanceDef::new(Some(8))),
Expand All @@ -139,12 +141,12 @@ impl BuiltinsInstanceDef {
pub(crate) fn all_cairo() -> BuiltinsInstanceDef {
BuiltinsInstanceDef {
output: true,
pedersen: Some(PedersenInstanceDef::new(Some(256), 1)),
pedersen: Some(PedersenInstanceDef::new(Some(256))),
range_check: Some(RangeCheckInstanceDef::default()),
ecdsa: Some(EcdsaInstanceDef::new(Some(2048))),
bitwise: Some(BitwiseInstanceDef::new(Some(16))),
ec_op: Some(EcOpInstanceDef::new(Some(1024))),
keccak: Some(KeccakInstanceDef::new(Some(2048), vec![200; 8])),
keccak: Some(KeccakInstanceDef::new(Some(2048))),
poseidon: Some(PoseidonInstanceDef::new(Some(256))),
range_check96: Some(RangeCheckInstanceDef::new(Some(8))),
#[cfg(feature = "mod_builtin")]
Expand Down Expand Up @@ -177,7 +179,7 @@ impl BuiltinsInstanceDef {
pub(crate) fn dynamic() -> BuiltinsInstanceDef {
BuiltinsInstanceDef {
output: true,
pedersen: Some(PedersenInstanceDef::new(None, 4)),
pedersen: Some(PedersenInstanceDef::new(None)),
range_check: Some(RangeCheckInstanceDef::new(None)),
ecdsa: Some(EcdsaInstanceDef::new(None)),
bitwise: Some(BitwiseInstanceDef::new(None)),
Expand Down
27 changes: 0 additions & 27 deletions vm/src/types/instance_definitions/cpu_instance_def.rs

This file was deleted.

Loading

0 comments on commit 1d23afb

Please sign in to comment.