Fix negative offset error. (lambdaclass#1271)

dojoengine · Jun 23, 2023 · 8054e33 · 8054e33
1 parent c329316
commit 8054e33
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,8 @@
 #### Upcoming Changes
 * 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)
 
+* fix: change error returned when subtracting two `MaybeRelocatable`s to better reflect the cause [#1271](https://github.com/lambdaclass/cairo-rs/pull/1271)
+
 #### [0.6.0] - 2023-6-18
 
 * fix: `dibit` hint no longer fails when called with an `m` of zero [#1247](https://github.com/lambdaclass/cairo-rs/pull/1247)

diff --git a/vm/src/hint_processor/builtin_hint_processor/blake2s_utils.rs b/vm/src/hint_processor/builtin_hint_processor/blake2s_utils.rs
@@ -340,7 +340,7 @@ mod tests {
         //Execute the hint
         assert_matches!(
             run_hint!(vm, ids_data, hint_code),
-            Err(HintError::Math(MathError::RelocatableSubNegOffset(bx)))
+            Err(HintError::Math(MathError::RelocatableSubUsizeNegOffset(bx)))
             if *bx == (relocatable!(2,5), 26)
         );
     }

diff --git a/vm/src/types/errors/math_errors.rs b/vm/src/types/errors/math_errors.rs
@@ -37,7 +37,9 @@ pub enum MathError {
     #[error("Cant convert felt: {0} to Relocatable")]
     Felt252ToRelocatable(Box<Felt252>),
     #[error("Operation failed: {} - {}, offsets cant be negative", (*.0).0, (*.0).1)]
-    RelocatableSubNegOffset(Box<(Relocatable, usize)>),
+    RelocatableSubFelt252NegOffset(Box<(Relocatable, Felt252)>),
+    #[error("Operation failed: {} - {}, offsets cant be negative", (*.0).0, (*.0).1)]
+    RelocatableSubUsizeNegOffset(Box<(Relocatable, usize)>),
     #[error("Operation failed: {} + {}, maximum offset value exceeded", (*.0).0, (*.0).1)]
     RelocatableAddFelt252OffsetExceeded(Box<(Relocatable, Felt252)>),
     #[error("Operation failed: {} + {}, maximum offset value exceeded", (*.0).0, (*.0).1)]

diff --git a/vm/src/types/relocatable.rs b/vm/src/types/relocatable.rs
@@ -147,7 +147,9 @@ impl Sub<usize> for Relocatable {
     type Output = Result<Relocatable, MathError>;
     fn sub(self, other: usize) -> Result<Self, MathError> {
         if self.offset < other {
-            return Err(MathError::RelocatableSubNegOffset(Box::new((self, other))));
+            return Err(MathError::RelocatableSubUsizeNegOffset(Box::new((
+                self, other,
+            ))));
         }
         let new_offset = self.offset - other;
         Ok(relocatable!(self.segment_index, new_offset))
@@ -161,7 +163,7 @@ impl Sub<Relocatable> for Relocatable {
             return Err(MathError::RelocatableSubDiffIndex(Box::new((self, other))));
         }
         if self.offset < other.offset {
-            return Err(MathError::RelocatableSubNegOffset(Box::new((
+            return Err(MathError::RelocatableSubUsizeNegOffset(Box::new((
                 self,
                 other.offset,
             ))));
@@ -268,10 +270,7 @@ impl MaybeRelocatable {
                 Ok(MaybeRelocatable::from((
                     rel_a.segment_index,
                     (rel_a.offset - num_b).to_usize().ok_or_else(|| {
-                        MathError::RelocatableAddFelt252OffsetExceeded(Box::new((
-                            *rel_a,
-                            num_b.clone(),
-                        )))
+                        MathError::RelocatableSubFelt252NegOffset(Box::new((*rel_a, num_b.clone())))
                     })?,
                 )))
             }
@@ -789,7 +788,7 @@ mod tests {
 
         assert_eq!(
             reloc + (-3),
-            Err(MathError::RelocatableSubNegOffset(Box::new((
+            Err(MathError::RelocatableSubUsizeNegOffset(Box::new((
                 relocatable!(1, 1),
                 3
             ))))
@@ -814,7 +813,7 @@ mod tests {
         assert_eq!(reloc - relocatable!(7, 5), Ok(1));
         assert_eq!(
             reloc - relocatable!(7, 9),
-            Err(MathError::RelocatableSubNegOffset(Box::new((
+            Err(MathError::RelocatableSubUsizeNegOffset(Box::new((
                 relocatable!(7, 6),
                 9
             ))))