Skip to content

Commit cf5768c

Browse files
MegaRedHandOppen
MegaRedHand
and
Oppen
authored
feat: wrap big variants of errors in Box (#1193)
* Wrap HintError's variants' contents in Box * Fix half of the errors * Fix compile warnings * Use `crate::stdlib`'s `Box` * Appease clippy * Add simple smoke tests of message formatting * Update changelog * Mention change is breaking * Box `MathError` variants * Box `MemoryError` variants * Box HintError variants (now also strings) * Add tentative test to avoid size regressions * Box `VirtualMachineError` * Box RunnerError * Appease clippy * Use `crate::stdlib`'s `Box` * Update changelog with recent changes * fix: replace several ok_ok by ok_or_else for perf regression * Replace more ok_or for ok_or_else * Add missing regression test * Use `Box<str>` instead of `Box<String>` --------- Co-authored-by: Mario Rugiero <[email protected]>
1 parent d9784bf commit cf5768c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

59 files changed

+1289
-1194
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
## Cairo-VM Changelog
22

33
#### Upcoming Changes
4+
5+
* feat: wrap big variants of `HintError`, `VirtualMachineError`, `RunnerError`, `MemoryError`, `MathError`, `InsufficientAllocatedCellsError` in `Box` [#1193](https://github.com/lambdaclass/cairo-rs/pull/1193)
6+
* BREAKING: all tuple variants of `HintError` with a single `Felt252` or multiple elements now receive a single `Box`
7+
48
* Add `Program::builtins_len method` [#1194](https://github.com/lambdaclass/cairo-rs/pull/1194)
59

610
* 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)

‎src/hint_processor/builtin_hint_processor/blake2s_utils.rs‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ pub fn example_blake2s_compress(
285285
let n_bytes = get_integer_from_var_name("n_bytes", vm, ids_data, ap_tracking).map(|x| {
286286
x.to_u32()
287287
.ok_or(HintError::Math(MathError::Felt252ToU32Conversion(
288-
x.into_owned(),
288+
Box::new(x.into_owned()),
289289
)))
290290
})??;
291291

@@ -339,10 +339,8 @@ mod tests {
339339
//Execute the hint
340340
assert_matches!(
341341
run_hint!(vm, ids_data, hint_code),
342-
Err(HintError::Math(MathError::RelocatableSubNegOffset(
343-
x,
344-
y
345-
))) if x == relocatable!(2,5) && y == 26
342+
Err(HintError::Math(MathError::RelocatableSubNegOffset(bx)))
343+
if *bx == (relocatable!(2,5), 26)
346344
);
347345
}
348346

@@ -363,8 +361,8 @@ mod tests {
363361
assert_matches!(
364362
run_hint!(vm, ids_data, hint_code),
365363
Err(HintError::Memory(MemoryError::UnknownMemoryCell(
366-
x
367-
))) if x == Relocatable::from((2, 0))
364+
bx
365+
))) if *bx == Relocatable::from((2, 0))
368366
);
369367
}
370368

@@ -383,8 +381,8 @@ mod tests {
383381
//Execute the hint
384382
assert_matches!(
385383
run_hint!(vm, ids_data, hint_code),
386-
Err(HintError::IdentifierNotRelocatable(x, y)
387-
) if x == "output" && y == (1,0).into()
384+
Err(HintError::IdentifierNotRelocatable(bx))
385+
if *bx == ("output".to_string(), (1,0).into())
388386
);
389387
}
390388

@@ -433,8 +431,8 @@ mod tests {
433431
assert_matches!(
434432
run_hint!(vm, ids_data, hint_code),
435433
Err(HintError::Memory(MemoryError::ExpectedInteger(
436-
x
437-
))) if x == Relocatable::from((2, 0))
434+
bx
435+
))) if *bx == Relocatable::from((2, 0))
438436
);
439437
}
440438

@@ -500,14 +498,10 @@ mod tests {
500498
assert_matches!(
501499
run_hint!(vm, ids_data, hint_code),
502500
Err(HintError::Memory(
503-
MemoryError::InconsistentMemory(
504-
x,
505-
y,
506-
z
507-
)
508-
)) if x == Relocatable::from((2, 0)) &&
509-
y == MaybeRelocatable::from((2, 0)) &&
510-
z == MaybeRelocatable::from(Felt252::new(1795745351))
501+
MemoryError::InconsistentMemory(bx)
502+
)) if *bx == (Relocatable::from((2, 0)),
503+
MaybeRelocatable::from((2, 0)),
504+
MaybeRelocatable::from(Felt252::new(1795745351)))
511505
);
512506
}
513507

@@ -522,7 +516,7 @@ mod tests {
522516
//Execute the hint
523517
assert_matches!(
524518
run_hint!(vm, HashMap::new(), hint_code),
525-
Err(HintError::UnknownIdentifier(x)) if x == "blake2s_ptr_end"
519+
Err(HintError::UnknownIdentifier(bx)) if bx.as_ref() == "blake2s_ptr_end"
526520
);
527521
}
528522

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

675669
#[test]
@@ -683,6 +677,6 @@ mod tests {
683677
vm.segments = segments![((1, 0), 9), ((1, 1), (1, 0)), ((1, 2), (2, 0))];
684678
let ids_data = ids_data!["n_bytes", "output", "blake2s_start"];
685679
//Execute the hint
686-
assert_matches!(run_hint!(vm, ids_data, hint_code::EXAMPLE_BLAKE2S_COMPRESS), Err(HintError::Memory(MemoryError::UnknownMemoryCell(x))) if x == (2, 0).into());
680+
assert_matches!(run_hint!(vm, ids_data, hint_code::EXAMPLE_BLAKE2S_COMPRESS), Err(HintError::Memory(MemoryError::UnknownMemoryCell(bx))) if *bx == (2, 0).into());
687681
}
688682
}

‎src/hint_processor/builtin_hint_processor/builtin_hint_processor_definition.rs‎

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,7 @@ impl HintProcessor for BuiltinHintProcessor {
796796
hint_code::SPLIT_XX => split_xx(vm, &hint_data.ids_data, &hint_data.ap_tracking),
797797
#[cfg(feature = "skip_next_instruction_hint")]
798798
hint_code::SKIP_NEXT_INSTRUCTION => skip_next_instruction(vm),
799-
code => Err(HintError::UnknownHint(code.to_string())),
799+
code => Err(HintError::UnknownHint(code.to_string().into_boxed_str())),
800800
}
801801
}
802802
}
@@ -865,18 +865,13 @@ mod tests {
865865
add_segments!(vm, 1);
866866
//ids and references are not needed for this test
867867
assert_matches!(
868-
run_hint!(vm, HashMap::new(), hint_code),
869-
Err(HintError::Memory(
870-
MemoryError::InconsistentMemory(
871-
x,
872-
y,
873-
z
874-
)
875-
)) if x ==
876-
Relocatable::from((1, 6)) &&
877-
y == MaybeRelocatable::from((1, 6)) &&
878-
z == MaybeRelocatable::from((3, 0))
879-
);
868+
run_hint!(vm, HashMap::new(), hint_code),
869+
Err(HintError::Memory(
870+
MemoryError::InconsistentMemory(bx)
871+
)) if *bx == (Relocatable::from((1, 6)),
872+
MaybeRelocatable::from((1, 6)),
873+
MaybeRelocatable::from((3, 0)))
874+
);
880875
}
881876

882877
#[test]
@@ -886,7 +881,7 @@ mod tests {
886881
let mut vm = vm!();
887882
assert_matches!(
888883
run_hint!(vm, HashMap::new(), hint_code),
889-
Err(HintError::UnknownHint(x)) if x == *hint_code.to_string()
884+
Err(HintError::UnknownHint(bx)) if bx.as_ref() == hint_code
890885
);
891886
}
892887

@@ -921,8 +916,8 @@ mod tests {
921916
let ids_data = ids_data!["len"];
922917
assert_matches!(
923918
run_hint!(vm, ids_data, hint_code),
924-
Err(HintError::IdentifierNotInteger(x, y))
925-
if x == "len" && y == (1,1).into()
919+
Err(HintError::IdentifierNotInteger(bx))
920+
if *bx == ("len".to_string(), (1,1).into())
926921
);
927922
}
928923

@@ -959,7 +954,7 @@ mod tests {
959954
let ids_data = ids_data!["continue_copying"];
960955
assert_matches!(
961956
run_hint!(vm, ids_data, hint_code),
962-
Err(HintError::VariableNotInScopeError(x)) if x == *"n".to_string()
957+
Err(HintError::VariableNotInScopeError(bx)) if bx.as_ref() == "n"
963958
);
964959
}
965960

@@ -980,18 +975,14 @@ mod tests {
980975

981976
let ids_data = ids_data!["continue_copying"];
982977
assert_matches!(
983-
run_hint!(vm, ids_data, hint_code, &mut exec_scopes),
984-
Err(HintError::Memory(
985-
MemoryError::InconsistentMemory(
986-
x,
987-
y,
988-
z
989-
)
990-
)) if x ==
991-
Relocatable::from((1, 1)) &&
992-
y == MaybeRelocatable::from(Felt252::new(5)) &&
993-
z == MaybeRelocatable::from(Felt252::zero())
994-
);
978+
run_hint!(vm, ids_data, hint_code, &mut exec_scopes),
979+
Err(HintError::Memory(
980+
MemoryError::InconsistentMemory(bx)
981+
)) if *bx ==
982+
(Relocatable::from((1, 1)),
983+
MaybeRelocatable::from(Felt252::new(5)),
984+
MaybeRelocatable::from(Felt252::zero()))
985+
);
995986
}
996987

997988
#[test]
@@ -1086,7 +1077,7 @@ mod tests {
10861077
let mut exec_scopes = scope![("__keccak_max_size", Felt252::new(2))];
10871078
assert_matches!(
10881079
run_hint!(vm, ids_data, hint_code, &mut exec_scopes),
1089-
Err(HintError::KeccakMaxSize(x, y)) if x == Felt252::new(5) && y == Felt252::new(2)
1080+
Err(HintError::KeccakMaxSize(bx)) if *bx == (Felt252::new(5), Felt252::new(2))
10901081
);
10911082
}
10921083

@@ -1134,7 +1125,7 @@ mod tests {
11341125
let mut exec_scopes = scope![("__keccak_max_size", Felt252::new(10))];
11351126
assert_matches!(
11361127
run_hint!(vm, ids_data, hint_code, &mut exec_scopes),
1137-
Err(HintError::InvalidWordSize(x)) if x == Felt252::new(-1)
1128+
Err(HintError::InvalidWordSize(bx)) if *bx == Felt252::new(-1)
11381129
);
11391130
}
11401131

‎src/hint_processor/builtin_hint_processor/cairo_keccak/keccak_hints.rs‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::stdlib::{
22
borrow::{Cow, ToOwned},
3+
boxed::Box,
34
collections::HashMap,
45
prelude::*,
56
};
@@ -93,7 +94,7 @@ pub fn compare_bytes_in_word_nondet(
9394
// Felt252::new(BYTES_INTO_WORD) into a lazy_static!
9495
let bytes_in_word = constants
9596
.get(BYTES_IN_WORD)
96-
.ok_or(HintError::MissingConstant(BYTES_IN_WORD))?;
97+
.ok_or_else(|| HintError::MissingConstant(Box::new(BYTES_IN_WORD)))?;
9798
let value = Felt252::new((n_bytes < bytes_in_word) as usize);
9899
insert_value_into_ap(vm, value)
99100
}
@@ -118,7 +119,7 @@ pub fn compare_keccak_full_rate_in_bytes_nondet(
118119
let keccak_full_rate_in_bytes = constants
119120
.get(KECCAK_FULL_RATE_IN_BYTES_CAIRO_KECCAK)
120121
.or_else(|| constants.get(KECCAK_FULL_RATE_IN_BYTES_BUILTIN_KECCAK))
121-
.ok_or(HintError::MissingConstant(KECCAK_FULL_RATE_IN_BYTES))?;
122+
.ok_or_else(|| HintError::MissingConstant(Box::new(KECCAK_FULL_RATE_IN_BYTES)))?;
122123
let value = Felt252::new((n_bytes >= keccak_full_rate_in_bytes) as usize);
123124
insert_value_into_ap(vm, value)
124125
}
@@ -152,11 +153,11 @@ pub(crate) fn block_permutation_v1(
152153
) -> Result<(), HintError> {
153154
let keccak_state_size_felts = constants
154155
.get(KECCAK_STATE_SIZE_FELTS)
155-
.ok_or(HintError::MissingConstant(KECCAK_STATE_SIZE_FELTS))?;
156+
.ok_or_else(|| HintError::MissingConstant(Box::new(KECCAK_STATE_SIZE_FELTS)))?;
156157
if keccak_state_size_felts >= &Felt252::new(100_i32) {
157-
return Err(HintError::InvalidKeccakStateSizeFelt252s(
158+
return Err(HintError::InvalidKeccakStateSizeFelt252s(Box::new(
158159
keccak_state_size_felts.clone(),
159-
));
160+
)));
160161
}
161162

162163
let keccak_ptr = get_ptr_from_var_name("keccak_ptr", vm, ids_data, ap_tracking)?;
@@ -219,11 +220,11 @@ pub(crate) fn block_permutation_v2(
219220
) -> Result<(), HintError> {
220221
let keccak_state_size_felts = constants
221222
.get(KECCAK_STATE_SIZE_FELTS)
222-
.ok_or(HintError::MissingConstant(KECCAK_STATE_SIZE_FELTS))?;
223+
.ok_or_else(|| HintError::MissingConstant(Box::new(KECCAK_STATE_SIZE_FELTS)))?;
223224
if keccak_state_size_felts >= &Felt252::from(100_i32) {
224-
return Err(HintError::InvalidKeccakStateSizeFelt252s(
225+
return Err(HintError::InvalidKeccakStateSizeFelt252s(Box::new(
225226
keccak_state_size_felts.clone(),
226-
));
227+
)));
227228
}
228229

229230
let keccak_ptr = get_ptr_from_var_name("keccak_ptr_start", vm, ids_data, ap_tracking)?;
@@ -256,19 +257,19 @@ fn cairo_keccak_finalize(
256257
) -> Result<(), HintError> {
257258
let keccak_state_size_felts = constants
258259
.get(KECCAK_STATE_SIZE_FELTS)
259-
.ok_or(HintError::MissingConstant(KECCAK_STATE_SIZE_FELTS))?;
260+
.ok_or_else(|| HintError::MissingConstant(Box::new(KECCAK_STATE_SIZE_FELTS)))?;
260261
let block_size = constants
261262
.get(BLOCK_SIZE)
262-
.ok_or(HintError::MissingConstant(BLOCK_SIZE))?;
263+
.ok_or_else(|| HintError::MissingConstant(Box::new(BLOCK_SIZE)))?;
263264

264265
if keccak_state_size_felts >= &Felt252::new(100_i32) {
265-
return Err(HintError::InvalidKeccakStateSizeFelt252s(
266+
return Err(HintError::InvalidKeccakStateSizeFelt252s(Box::new(
266267
keccak_state_size_felts.clone(),
267-
));
268+
)));
268269
}
269270

270271
if block_size >= &Felt252::new(block_size_limit) {
271-
return Err(HintError::InvalidBlockSize(block_size.clone()));
272+
return Err(HintError::InvalidBlockSize(Box::new(block_size.clone())));
272273
};
273274

274275
let keccak_state_size_felts = keccak_state_size_felts.to_usize().unwrap();
@@ -349,10 +350,10 @@ pub(crate) fn maybe_reloc_vec_to_u64_array(
349350
Some(Cow::Owned(MaybeRelocatable::Int(ref num)))
350351
| Some(Cow::Borrowed(MaybeRelocatable::Int(ref num))) => num
351352
.to_u64()
352-
.ok_or_else(|| MathError::Felt252ToU64Conversion(num.clone()).into()),
353-
_ => Err(VirtualMachineError::ExpectedIntAtRange(
353+
.ok_or_else(|| MathError::Felt252ToU64Conversion(Box::new(num.clone())).into()),
354+
_ => Err(VirtualMachineError::ExpectedIntAtRange(Box::new(
354355
n.as_ref().map(|x| x.as_ref().to_owned()),
355-
)),
356+
))),
356357
})
357358
.collect::<Result<Vec<u64>, VirtualMachineError>>()?;
358359

0 commit comments

Comments
 (0)