Skip to content
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.

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

Simplify Memory::get return type to Option #852

Merged
merged 8 commits into from
Feb 23, 2023
44 changes: 24 additions & 20 deletions src/hint_processor/builtin_hint_processor/blake2s_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,9 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell(
x
))) if x == MaybeRelocatable::from((2, 0))
))) if x == Relocatable::from((2, 0))
);
}

Expand All @@ -292,7 +292,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(
VirtualMachineError::ExpectedRelocatable(x)
)) if x == MaybeRelocatable::from((1, 0))
)) if x == Relocatable::from((1, 0))
);
}

Expand Down Expand Up @@ -340,7 +340,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((2, 0))
))) if x == Relocatable::from((2, 0))
);
}

Expand Down Expand Up @@ -454,10 +454,11 @@ mod tests {
((2, 6), 0),
((2, 7), 0)
];
assert_eq!(
vm.segments.memory.get(&MaybeRelocatable::from((2, 8))),
Ok(None)
);
assert!(vm
.segments
.memory
.get(&MaybeRelocatable::from((2, 8)))
.is_none());
}

#[test]
Expand Down Expand Up @@ -485,10 +486,11 @@ mod tests {
((2, 6), 0),
((2, 7), 0)
];
assert_eq!(
vm.segments.memory.get(&MaybeRelocatable::from((2, 8))),
Ok(None)
);
assert!(vm
.segments
.memory
.get(&MaybeRelocatable::from((2, 8)))
.is_none());
}

#[test]
Expand Down Expand Up @@ -516,10 +518,11 @@ mod tests {
((2, 6), 0),
((2, 7), 0)
];
assert_eq!(
vm.segments.memory.get(&MaybeRelocatable::from((2, 8))),
Ok(None)
);
assert!(vm
.segments
.memory
.get(&MaybeRelocatable::from((2, 8)))
.is_none());
}

#[test]
Expand Down Expand Up @@ -547,9 +550,10 @@ mod tests {
((2, 6), 0),
((2, 7), 20)
];
assert_eq!(
vm.segments.memory.get(&MaybeRelocatable::from((2, 8))),
Ok(None)
);
assert!(vm
.segments
.memory
.get(&MaybeRelocatable::from((2, 8)))
.is_none());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ impl HintProcessor for BuiltinHintProcessor {
#[cfg(test)]
mod tests {
use super::*;
use crate::types::relocatable::Relocatable;
use crate::vm::vm_memory::memory_segments::MemorySegmentManager;
use crate::{
any_box,
Expand Down Expand Up @@ -559,7 +560,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 1))
))) if x == Relocatable::from((1, 1))
);
}

Expand Down Expand Up @@ -802,7 +803,9 @@ mod tests {
let ids_data = non_continuous_ids_data![("keccak_state", -7), ("high", -3), ("low", -2)];
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::NoneInMemoryRange))
Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell(
_
)))
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,6 @@ mod tests {
.memory
.get(&MaybeRelocatable::from((1, 1)))
.unwrap()
.unwrap()
.as_ref(),
&MaybeRelocatable::from(12)
);
Expand Down
22 changes: 12 additions & 10 deletions src/hint_processor/builtin_hint_processor/find_element_hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,9 @@ mod tests {
let ids_data = ids_data!["array_ptr", "elm_size", "n_elms", "index", "key"];
assert_matches!(
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell(
x
))) if x == MaybeRelocatable::from((1, 4))
))) if x == Relocatable::from((1, 4))
);
}

Expand All @@ -285,7 +285,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 1))
))) if x == Relocatable::from((1, 1))
);
}

Expand Down Expand Up @@ -315,9 +315,11 @@ mod tests {

#[test]
fn find_elm_not_int_n_elms() {
let relocatable = MaybeRelocatable::from((1, 2));
let (mut vm, ids_data) =
init_vm_ids_data(HashMap::from([("n_elms".to_string(), relocatable.clone())]));
let relocatable = Relocatable::from((1, 2));
let (mut vm, ids_data) = init_vm_ids_data(HashMap::from([(
"n_elms".to_string(),
MaybeRelocatable::from(relocatable),
)]));
assert_matches!(
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
Expand Down Expand Up @@ -362,10 +364,10 @@ mod tests {
assert_matches!(
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
MaybeRelocatable::RelocatableValue(Relocatable {
Relocatable {
segment_index: 1,
offset: 4
})
}
)))
);
}
Expand Down Expand Up @@ -404,7 +406,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 1))
))) if x == Relocatable::from((1, 1))
);
}

Expand Down Expand Up @@ -442,7 +444,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 2))
))) if x == Relocatable::from((1, 2))
);
}

Expand Down
4 changes: 2 additions & 2 deletions src/hint_processor/builtin_hint_processor/hint_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ mod tests {
get_ptr_from_var_name("value", &vm, &ids_data, &ApTracking::new()),
Err(HintError::Internal(
VirtualMachineError::ExpectedRelocatable(x)
)) if x == MaybeRelocatable::from((1, 0))
)) if x == Relocatable::from((1, 0))
);
}

Expand Down Expand Up @@ -249,7 +249,7 @@ mod tests {
get_integer_from_var_name("value", &vm, &ids_data, &ApTracking::new()),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 0))
))) if x == Relocatable::from((1, 0))
);
}
}
41 changes: 5 additions & 36 deletions src/hint_processor/builtin_hint_processor/keccak_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,8 @@ use crate::{
hint_processor_definition::HintReference,
},
serde::deserialize_program::ApTracking,
types::{
exec_scope::ExecutionScopes,
relocatable::{MaybeRelocatable, Relocatable},
},
vm::{
errors::{hint_errors::HintError, vm_errors::VirtualMachineError},
vm_core::VirtualMachine,
},
types::{exec_scope::ExecutionScopes, relocatable::Relocatable},
vm::{errors::hint_errors::HintError, vm_core::VirtualMachine},
};
use felt::Felt;
use num_traits::{One, Signed, ToPrimitive};
Expand Down Expand Up @@ -150,27 +144,12 @@ pub fn unsafe_keccak_finalize(
offset: keccak_state_ptr.offset + 1,
})?;

// this is not very nice code, we should consider adding the sub() method for Relocatable's
let maybe_rel_start_ptr = MaybeRelocatable::RelocatableValue(start_ptr);
let maybe_rel_end_ptr = MaybeRelocatable::RelocatableValue(end_ptr);

let n_elems = maybe_rel_end_ptr
.sub(&maybe_rel_start_ptr)?
.get_int_ref()?
.to_usize()
.ok_or(VirtualMachineError::BigintToUsizeFail)?;
let n_elems = end_ptr.sub(&start_ptr)?;

let mut keccak_input = Vec::new();
let range = vm
.get_range(&maybe_rel_start_ptr, n_elems)
.map_err(VirtualMachineError::MemoryError)?;

check_no_nones_in_range(&range)?;

for maybe_reloc_word in range.into_iter() {
let word = maybe_reloc_word.ok_or(VirtualMachineError::ExpectedIntAtRange(None))?;
let word = word.get_int_ref()?;
let range = vm.get_integer_range(start_ptr, n_elems)?;

for word in range.into_iter() {
let mut bytes = word.to_bytes_be();
let mut bytes = {
let n_word_bytes = &bytes.len();
Expand Down Expand Up @@ -208,13 +187,3 @@ pub(crate) fn left_pad_u64(bytes_vector: &mut [u64], n_zeros: usize) -> Vec<u64>

res
}

fn check_no_nones_in_range<T>(range: &Vec<Option<T>>) -> Result<(), VirtualMachineError> {
for memory_cell in range {
memory_cell
.as_ref()
.ok_or(VirtualMachineError::NoneInMemoryRange)?;
}

Ok(())
}
32 changes: 16 additions & 16 deletions src/hint_processor/builtin_hint_processor/math_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ pub fn assert_not_equal(
let b_addr = get_address_from_var_name("b", vm, ids_data, ap_tracking)?;
//Check that the ids are in memory
match (vm.get_maybe(&a_addr), vm.get_maybe(&b_addr)) {
(Ok(Some(maybe_rel_a)), Ok(Some(maybe_rel_b))) => {
(Some(maybe_rel_a), Some(maybe_rel_b)) => {
let maybe_rel_a = maybe_rel_a;
let maybe_rel_b = maybe_rel_b;
match (maybe_rel_a, maybe_rel_b) {
Expand Down Expand Up @@ -672,9 +672,9 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell(
x
))) if x == MaybeRelocatable::from((1, 4))
))) if x == Relocatable::from((1, 4))
);
}

Expand All @@ -693,7 +693,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 4))
))) if x == Relocatable::from((1, 4))
);
}

Expand Down Expand Up @@ -840,9 +840,9 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell(
x
))) if x == MaybeRelocatable::from((1, 3))
))) if x == Relocatable::from((1, 3))
);
}

Expand Down Expand Up @@ -875,9 +875,9 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell(
x
))) if x == MaybeRelocatable::from((1, 3))
))) if x == Relocatable::from((1, 3))
);
}

Expand Down Expand Up @@ -932,7 +932,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code, &mut exec_scopes, &constants),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 0))
))) if x == Relocatable::from((1, 0))
);
}

Expand Down Expand Up @@ -960,7 +960,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code, &mut exec_scopes, &constants),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 1))
))) if x == Relocatable::from((1, 1))
);
}

Expand Down Expand Up @@ -1193,7 +1193,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 4))
))) if x == Relocatable::from((1, 4))
);
}

Expand Down Expand Up @@ -1791,7 +1791,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 3))
))) if x == Relocatable::from((1, 3))
);
}

Expand Down Expand Up @@ -1857,7 +1857,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 1))
))) if x == Relocatable::from((1, 1))
);
}

Expand All @@ -1875,7 +1875,7 @@ mod tests {
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
x
))) if x == MaybeRelocatable::from((1, 2))
))) if x == Relocatable::from((1, 2))
);
}

Expand All @@ -1892,9 +1892,9 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Internal(VirtualMachineError::ExpectedInteger(
Err(HintError::Internal(VirtualMachineError::UnknownMemoryCell(
x
))) if x == MaybeRelocatable::from((1, 2))
))) if x == Relocatable::from((1, 2))
);
}
}
Loading