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

feat: wrap big variants of errors in Box #1193

Merged
merged 24 commits into from
Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
76fc1cf
Wrap HintError's variants' contents in Box
MegaRedHand May 30, 2023
ae3034b
Fix half of the errors
MegaRedHand May 31, 2023
58f91cf
Fix compile warnings
MegaRedHand May 31, 2023
d16c5ed
Use `crate::stdlib`'s `Box`
MegaRedHand May 31, 2023
443362a
Appease clippy
MegaRedHand May 31, 2023
a824d87
Add simple smoke tests of message formatting
MegaRedHand May 31, 2023
f3a68fc
Merge branch 'main' into hinterror-boxing
MegaRedHand May 31, 2023
1a7c490
Update changelog
MegaRedHand May 31, 2023
d809fed
Mention change is breaking
MegaRedHand May 31, 2023
a80ed68
Merge branch 'main' into hinterror-boxing
MegaRedHand May 31, 2023
c95469d
Box `MathError` variants
MegaRedHand May 31, 2023
3e32c8f
Box `MemoryError` variants
MegaRedHand May 31, 2023
1a2c87d
Box HintError variants (now also strings)
MegaRedHand May 31, 2023
514def1
Add tentative test to avoid size regressions
MegaRedHand May 31, 2023
123414a
Box `VirtualMachineError`
MegaRedHand Jun 1, 2023
fb0519c
Box RunnerError
MegaRedHand Jun 1, 2023
13fbd9c
Appease clippy
MegaRedHand Jun 1, 2023
68f902d
Use `crate::stdlib`'s `Box`
MegaRedHand Jun 1, 2023
a9dfdf4
Update changelog with recent changes
MegaRedHand Jun 1, 2023
fea6ff2
fix: replace several ok_ok by ok_or_else for perf regression
Oppen Jun 1, 2023
f75d6c9
Replace more ok_or for ok_or_else
MegaRedHand Jun 1, 2023
506bf3b
Add missing regression test
MegaRedHand Jun 1, 2023
c9b5ef1
Use `Box<str>` instead of `Box<String>`
MegaRedHand Jun 1, 2023
8f91da3
Merge branch 'main' into hinterror-boxing
MegaRedHand Jun 1, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
## Cairo-VM Changelog

#### Upcoming Changes

* feat: wrap big variants of `HintError`, `VirtualMachineError`, `RunnerError`, `MemoryError`, `MathError`, `InsufficientAllocatedCellsError` in `Box` [#1193](https://github.com/lambdaclass/cairo-rs/pull/1193)
* BREAKING: all tuple variants of `HintError` with a single `Felt252` or multiple elements now receive a single `Box`

* Add `Program::builtins_len method` [#1194](https://github.com/lambdaclass/cairo-rs/pull/1194)

* fix: Handle the deserialization of serde_json::Number with scientific notation (e.g.: Number(1e27)) in felt_from_number function [#1188](https://github.com/lambdaclass/cairo-rs/pull/1188)
Expand Down
38 changes: 16 additions & 22 deletions src/hint_processor/builtin_hint_processor/blake2s_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ pub fn example_blake2s_compress(
let n_bytes = get_integer_from_var_name("n_bytes", vm, ids_data, ap_tracking).map(|x| {
x.to_u32()
.ok_or(HintError::Math(MathError::Felt252ToU32Conversion(
x.into_owned(),
Box::new(x.into_owned()),
)))
})??;

Expand Down Expand Up @@ -339,10 +339,8 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Math(MathError::RelocatableSubNegOffset(
x,
y
))) if x == relocatable!(2,5) && y == 26
Err(HintError::Math(MathError::RelocatableSubNegOffset(bx)))
if *bx == (relocatable!(2,5), 26)
);
}

Expand All @@ -363,8 +361,8 @@ mod tests {
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Memory(MemoryError::UnknownMemoryCell(
x
))) if x == Relocatable::from((2, 0))
bx
))) if *bx == Relocatable::from((2, 0))
);
}

Expand All @@ -383,8 +381,8 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::IdentifierNotRelocatable(x, y)
) if x == "output" && y == (1,0).into()
Err(HintError::IdentifierNotRelocatable(bx))
if *bx == ("output".to_string(), (1,0).into())
);
}

Expand Down Expand Up @@ -433,8 +431,8 @@ mod tests {
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Memory(MemoryError::ExpectedInteger(
x
))) if x == Relocatable::from((2, 0))
bx
))) if *bx == Relocatable::from((2, 0))
);
}

Expand Down Expand Up @@ -500,14 +498,10 @@ mod tests {
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::Memory(
MemoryError::InconsistentMemory(
x,
y,
z
)
)) if x == Relocatable::from((2, 0)) &&
y == MaybeRelocatable::from((2, 0)) &&
z == MaybeRelocatable::from(Felt252::new(1795745351))
MemoryError::InconsistentMemory(bx)
)) if *bx == (Relocatable::from((2, 0)),
MaybeRelocatable::from((2, 0)),
MaybeRelocatable::from(Felt252::new(1795745351)))
);
}

Expand All @@ -522,7 +516,7 @@ mod tests {
//Execute the hint
assert_matches!(
run_hint!(vm, HashMap::new(), hint_code),
Err(HintError::UnknownIdentifier(x)) if x == "blake2s_ptr_end"
Err(HintError::UnknownIdentifier(bx)) if *bx == "blake2s_ptr_end"
);
}

Expand Down Expand Up @@ -669,7 +663,7 @@ mod tests {
vm.segments = segments![((1, 0), 9999999999_u64), ((1, 1), (1, 0)), ((1, 2), (2, 0))];
let ids_data = ids_data!["n_bytes", "output", "blake2s_start"];
//Execute the hint
assert_matches!(run_hint!(vm, ids_data, hint_code::EXAMPLE_BLAKE2S_COMPRESS), Err(HintError::Math(MathError::Felt252ToU32Conversion(x))) if x == Felt252::from(9999999999_u64));
assert_matches!(run_hint!(vm, ids_data, hint_code::EXAMPLE_BLAKE2S_COMPRESS), Err(HintError::Math(MathError::Felt252ToU32Conversion(bx))) if *bx == Felt252::from(9999999999_u64));
}

#[test]
Expand All @@ -683,6 +677,6 @@ mod tests {
vm.segments = segments![((1, 0), 9), ((1, 1), (1, 0)), ((1, 2), (2, 0))];
let ids_data = ids_data!["n_bytes", "output", "blake2s_start"];
//Execute the hint
assert_matches!(run_hint!(vm, ids_data, hint_code::EXAMPLE_BLAKE2S_COMPRESS), Err(HintError::Memory(MemoryError::UnknownMemoryCell(x))) if x == (2, 0).into());
assert_matches!(run_hint!(vm, ids_data, hint_code::EXAMPLE_BLAKE2S_COMPRESS), Err(HintError::Memory(MemoryError::UnknownMemoryCell(bx))) if *bx == (2, 0).into());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,7 @@ impl HintProcessor for BuiltinHintProcessor {
hint_code::SPLIT_XX => split_xx(vm, &hint_data.ids_data, &hint_data.ap_tracking),
#[cfg(feature = "skip_next_instruction_hint")]
hint_code::SKIP_NEXT_INSTRUCTION => skip_next_instruction(vm),
code => Err(HintError::UnknownHint(code.to_string())),
code => Err(HintError::UnknownHint(Box::new(code.to_string()))),
}
}
}
Expand Down Expand Up @@ -865,18 +865,13 @@ mod tests {
add_segments!(vm, 1);
//ids and references are not needed for this test
assert_matches!(
run_hint!(vm, HashMap::new(), hint_code),
Err(HintError::Memory(
MemoryError::InconsistentMemory(
x,
y,
z
)
)) if x ==
Relocatable::from((1, 6)) &&
y == MaybeRelocatable::from((1, 6)) &&
z == MaybeRelocatable::from((3, 0))
);
run_hint!(vm, HashMap::new(), hint_code),
Err(HintError::Memory(
MemoryError::InconsistentMemory(bx)
)) if *bx == (Relocatable::from((1, 6)),
MaybeRelocatable::from((1, 6)),
MaybeRelocatable::from((3, 0)))
);
}

#[test]
Expand All @@ -886,7 +881,7 @@ mod tests {
let mut vm = vm!();
assert_matches!(
run_hint!(vm, HashMap::new(), hint_code),
Err(HintError::UnknownHint(x)) if x == *hint_code.to_string()
Err(HintError::UnknownHint(bx)) if *bx == hint_code
);
}

Expand Down Expand Up @@ -921,8 +916,8 @@ mod tests {
let ids_data = ids_data!["len"];
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::IdentifierNotInteger(x, y))
if x == "len" && y == (1,1).into()
Err(HintError::IdentifierNotInteger(bx))
if *bx == ("len".to_string(), (1,1).into())
);
}

Expand Down Expand Up @@ -959,7 +954,7 @@ mod tests {
let ids_data = ids_data!["continue_copying"];
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::VariableNotInScopeError(x)) if x == *"n".to_string()
Err(HintError::VariableNotInScopeError(bx)) if *bx == "n"
);
}

Expand All @@ -980,18 +975,14 @@ mod tests {

let ids_data = ids_data!["continue_copying"];
assert_matches!(
run_hint!(vm, ids_data, hint_code, &mut exec_scopes),
Err(HintError::Memory(
MemoryError::InconsistentMemory(
x,
y,
z
)
)) if x ==
Relocatable::from((1, 1)) &&
y == MaybeRelocatable::from(Felt252::new(5)) &&
z == MaybeRelocatable::from(Felt252::zero())
);
run_hint!(vm, ids_data, hint_code, &mut exec_scopes),
Err(HintError::Memory(
MemoryError::InconsistentMemory(bx)
)) if *bx ==
(Relocatable::from((1, 1)),
MaybeRelocatable::from(Felt252::new(5)),
MaybeRelocatable::from(Felt252::zero()))
);
}

#[test]
Expand Down Expand Up @@ -1086,7 +1077,7 @@ mod tests {
let mut exec_scopes = scope![("__keccak_max_size", Felt252::new(2))];
assert_matches!(
run_hint!(vm, ids_data, hint_code, &mut exec_scopes),
Err(HintError::KeccakMaxSize(x, y)) if x == Felt252::new(5) && y == Felt252::new(2)
Err(HintError::KeccakMaxSize(bx)) if *bx == (Felt252::new(5), Felt252::new(2))
);
}

Expand Down Expand Up @@ -1134,7 +1125,7 @@ mod tests {
let mut exec_scopes = scope![("__keccak_max_size", Felt252::new(10))];
assert_matches!(
run_hint!(vm, ids_data, hint_code, &mut exec_scopes),
Err(HintError::InvalidWordSize(x)) if x == Felt252::new(-1)
Err(HintError::InvalidWordSize(bx)) if *bx == Felt252::new(-1)
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::stdlib::{
borrow::{Cow, ToOwned},
boxed::Box,
collections::HashMap,
prelude::*,
};
Expand Down Expand Up @@ -93,7 +94,7 @@
// Felt252::new(BYTES_INTO_WORD) into a lazy_static!
let bytes_in_word = constants
.get(BYTES_IN_WORD)
.ok_or(HintError::MissingConstant(BYTES_IN_WORD))?;
.ok_or(HintError::MissingConstant(Box::new(BYTES_IN_WORD)))?;
let value = Felt252::new((n_bytes < bytes_in_word) as usize);
insert_value_into_ap(vm, value)
}
Expand All @@ -118,7 +119,9 @@
let keccak_full_rate_in_bytes = constants
.get(KECCAK_FULL_RATE_IN_BYTES_CAIRO_KECCAK)
.or_else(|| constants.get(KECCAK_FULL_RATE_IN_BYTES_BUILTIN_KECCAK))
.ok_or(HintError::MissingConstant(KECCAK_FULL_RATE_IN_BYTES))?;
.ok_or(HintError::MissingConstant(Box::new(
KECCAK_FULL_RATE_IN_BYTES,
)))?;
let value = Felt252::new((n_bytes >= keccak_full_rate_in_bytes) as usize);
insert_value_into_ap(vm, value)
}
Expand Down Expand Up @@ -150,13 +153,16 @@
ap_tracking: &ApTracking,
constants: &HashMap<String, Felt252>,
) -> Result<(), HintError> {
let keccak_state_size_felts = constants
.get(KECCAK_STATE_SIZE_FELTS)
.ok_or(HintError::MissingConstant(KECCAK_STATE_SIZE_FELTS))?;
let keccak_state_size_felts =
constants
.get(KECCAK_STATE_SIZE_FELTS)
.ok_or(HintError::MissingConstant(Box::new(
KECCAK_STATE_SIZE_FELTS,
)))?;
if keccak_state_size_felts >= &Felt252::new(100_i32) {
return Err(HintError::InvalidKeccakStateSizeFelt252s(
return Err(HintError::InvalidKeccakStateSizeFelt252s(Box::new(

Check warning on line 163 in src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs

View check run for this annotation

Codecov / codecov/patch

src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs#L163

Added line #L163 was not covered by tests
keccak_state_size_felts.clone(),
));
)));

Check warning on line 165 in src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs

View check run for this annotation

Codecov / codecov/patch

src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs#L165

Added line #L165 was not covered by tests
}

let keccak_ptr = get_ptr_from_var_name("keccak_ptr", vm, ids_data, ap_tracking)?;
Expand Down Expand Up @@ -217,13 +223,16 @@
ap_tracking: &ApTracking,
constants: &HashMap<String, Felt252>,
) -> Result<(), HintError> {
let keccak_state_size_felts = constants
.get(KECCAK_STATE_SIZE_FELTS)
.ok_or(HintError::MissingConstant(KECCAK_STATE_SIZE_FELTS))?;
let keccak_state_size_felts =
constants
.get(KECCAK_STATE_SIZE_FELTS)
.ok_or(HintError::MissingConstant(Box::new(
KECCAK_STATE_SIZE_FELTS,
)))?;
if keccak_state_size_felts >= &Felt252::from(100_i32) {
return Err(HintError::InvalidKeccakStateSizeFelt252s(
return Err(HintError::InvalidKeccakStateSizeFelt252s(Box::new(

Check warning on line 233 in src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs

View check run for this annotation

Codecov / codecov/patch

src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs#L233

Added line #L233 was not covered by tests
keccak_state_size_felts.clone(),
));
)));

Check warning on line 235 in src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs

View check run for this annotation

Codecov / codecov/patch

src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs#L235

Added line #L235 was not covered by tests
}

let keccak_ptr = get_ptr_from_var_name("keccak_ptr_start", vm, ids_data, ap_tracking)?;
Expand Down Expand Up @@ -254,21 +263,24 @@
constants: &HashMap<String, Felt252>,
block_size_limit: usize,
) -> Result<(), HintError> {
let keccak_state_size_felts = constants
.get(KECCAK_STATE_SIZE_FELTS)
.ok_or(HintError::MissingConstant(KECCAK_STATE_SIZE_FELTS))?;
let keccak_state_size_felts =
constants
.get(KECCAK_STATE_SIZE_FELTS)
.ok_or(HintError::MissingConstant(Box::new(
KECCAK_STATE_SIZE_FELTS,
)))?;
let block_size = constants
.get(BLOCK_SIZE)
.ok_or(HintError::MissingConstant(BLOCK_SIZE))?;
.ok_or(HintError::MissingConstant(Box::new(BLOCK_SIZE)))?;

if keccak_state_size_felts >= &Felt252::new(100_i32) {
return Err(HintError::InvalidKeccakStateSizeFelt252s(
return Err(HintError::InvalidKeccakStateSizeFelt252s(Box::new(

Check warning on line 277 in src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs

View check run for this annotation

Codecov / codecov/patch

src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs#L277

Added line #L277 was not covered by tests
keccak_state_size_felts.clone(),
));
)));

Check warning on line 279 in src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs

View check run for this annotation

Codecov / codecov/patch

src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs#L279

Added line #L279 was not covered by tests
}

if block_size >= &Felt252::new(block_size_limit) {
return Err(HintError::InvalidBlockSize(block_size.clone()));
return Err(HintError::InvalidBlockSize(Box::new(block_size.clone())));

Check warning on line 283 in src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs

View check run for this annotation

Codecov / codecov/patch

src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs#L283

Added line #L283 was not covered by tests
};

let keccak_state_size_felts = keccak_state_size_felts.to_usize().unwrap();
Expand Down Expand Up @@ -349,10 +361,10 @@
Some(Cow::Owned(MaybeRelocatable::Int(ref num)))
| Some(Cow::Borrowed(MaybeRelocatable::Int(ref num))) => num
.to_u64()
.ok_or_else(|| MathError::Felt252ToU64Conversion(num.clone()).into()),
_ => Err(VirtualMachineError::ExpectedIntAtRange(
.ok_or_else(|| MathError::Felt252ToU64Conversion(Box::new(num.clone())).into()),
_ => Err(VirtualMachineError::ExpectedIntAtRange(Box::new(

Check warning on line 365 in src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs

View check run for this annotation

Codecov / codecov/patch

src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs#L365

Added line #L365 was not covered by tests
n.as_ref().map(|x| x.as_ref().to_owned()),
)),
))),

Check warning on line 367 in src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs

View check run for this annotation

Codecov / codecov/patch

src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs#L367

Added line #L367 was not covered by tests
})
.collect::<Result<Vec<u64>, VirtualMachineError>>()?;

Expand Down
Loading
Loading