Skip to content

Commit

Permalink
Fix: use dedicated struct to store the output builtin state
Browse files Browse the repository at this point in the history
The `get_state` and `set_state` methods now rely on the new
`OutputBuiltinState` struct. Rolled back the introduction of the base
field in `OutputBuiltinAdditionalData`.
  • Loading branch information
odesenfans committed Jan 30, 2024
1 parent a9b8545 commit 9f3efea
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 10 deletions.
1 change: 0 additions & 1 deletion vm/src/tests/cairo_pie_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ fn pedersen_test() {
(
OUTPUT_BUILTIN_NAME.to_string(),
BuiltinAdditionalData::Output(OutputBuiltinAdditionalData {
base: 2,
pages: HashMap::new(),
attributes: HashMap::new(),
}),
Expand Down
30 changes: 23 additions & 7 deletions vm/src/vm/runners/builtin_runner/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ use crate::vm::vm_memory::memory_segments::MemorySegmentManager;

use super::OUTPUT_BUILTIN_NAME;

#[derive(Debug, Clone, PartialEq)]
pub struct OutputBuiltinState {
pub base: usize,
pub pages: Pages,
pub attributes: Attributes,
}

#[derive(Debug, Clone)]
pub struct OutputBuiltinRunner {
base: usize,
Expand Down Expand Up @@ -124,16 +131,23 @@ impl OutputBuiltinRunner {

pub fn get_additional_data(&self) -> BuiltinAdditionalData {
BuiltinAdditionalData::Output(OutputBuiltinAdditionalData {
base: self.base,
pages: self.pages.clone(),
attributes: self.attributes.clone(),
})
}

pub fn set_state(&mut self, data: OutputBuiltinAdditionalData) {
self.base = data.base;
self.pages = data.pages;
self.attributes = data.attributes;
pub fn set_state(&mut self, new_state: OutputBuiltinState) {
self.base = new_state.base;
self.pages = new_state.pages;
self.attributes = new_state.attributes;
}

pub fn get_state(&mut self) -> OutputBuiltinState {
OutputBuiltinState {
base: self.base,
pages: self.pages.clone(),
attributes: self.attributes.clone(),
}
}

pub fn add_page(
Expand Down Expand Up @@ -502,7 +516,6 @@ mod tests {
assert_eq!(
builtin.get_additional_data(),
BuiltinAdditionalData::Output(OutputBuiltinAdditionalData {
base: builtin.base,
pages: HashMap::default(),
attributes: HashMap::default()
})
Expand All @@ -523,7 +536,7 @@ mod tests {
let mut builtin = OutputBuiltinRunner::new(true);
assert_eq!(builtin.base, 0);

let new_state = OutputBuiltinAdditionalData {
let new_state = OutputBuiltinState {
base: 10,
pages: HashMap::from([(1, PublicMemoryPage { start: 0, size: 3 })]),
attributes: HashMap::from([("gps_fact_topology".to_string(), vec![0, 2, 0])]),
Expand All @@ -533,6 +546,9 @@ mod tests {
assert_eq!(builtin.base, new_state.base);
assert_eq!(builtin.pages, new_state.pages);
assert_eq!(builtin.attributes, new_state.attributes);

let state = builtin.get_state();
assert_eq!(state, new_state);
}

#[test]
Expand Down
2 changes: 0 additions & 2 deletions vm/src/vm/runners/cairo_pie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ pub type Pages = HashMap<usize, PublicMemoryPage>;

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
pub struct OutputBuiltinAdditionalData {
#[serde(skip)]
pub base: usize,
pub pages: Pages,
pub attributes: Attributes,
}
Expand Down

0 comments on commit 9f3efea

Please sign in to comment.