Skip to content

Commit

Permalink
fix: substraction between relocatables should be signed (#1218)
Browse files Browse the repository at this point in the history
* fix: substraction between relocatables should be signed

It is valid as per the reference implementation to substract two
relocatables `a - b` where `b.offset > a.offset`.
The fix consists in not using the trait as implemented for `Relocatable`
(where it represents a positive distance between pointers) but to
promote the offsets to `i128` and only substract them for
`MaybeRelocatable`, as those are the language-level types.

* chore: remove uneeded stuff

* Add fix holes to if_reloc_equal.cairo test

* chore: changelog

---------

Co-authored-by: Pedro Fontana <fontana.pedro93@gmail.com>
  • Loading branch information
Oppen and pefontana authored Jun 10, 2023
1 parent 5b406bf commit 2b7de08
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#### [0.5.1] - 2023-6-7

* fix: substraction of `MaybeRelocatable` always behaves as signed [#1218](https://github.com/lambdaclass/cairo-rs/pull/1218)

* fix: fix overflow for `QUAD_BIT` and `DI_BIT` hints [#1209](https://github.com/lambdaclass/cairo-rs/pull/1209)
Fixes [#1205](https://github.com/lambdaclass/cairo-rs/issue/1205)

Expand Down
16 changes: 16 additions & 0 deletions cairo_programs/if_reloc_equal.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from starkware.cairo.common.alloc import alloc

func main{}() {
let arr: felt* = alloc();

assert arr[0] = 1;
assert arr[5] = 2;

let end = arr + 5;

if (arr == end) {
ret;
}

ret;
}
6 changes: 6 additions & 0 deletions src/tests/cairo_run_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -967,3 +967,9 @@ fn cairo_run_reduce() {
let program_data = include_bytes!("../../cairo_programs/reduce.json");
run_program_simple(program_data.as_slice());
}

#[test]
fn cairo_run_if_reloc_equal() {
let program_data = include_bytes!("../../cairo_programs/if_reloc_equal.json");
run_program_simple_with_memory_holes(program_data, 4);
}
4 changes: 3 additions & 1 deletion src/types/relocatable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,9 @@ impl MaybeRelocatable {
MaybeRelocatable::RelocatableValue(rel_b),
) => {
if rel_a.segment_index == rel_b.segment_index {
return Ok(MaybeRelocatable::from(Felt252::new((*rel_a - *rel_b)?)));
return Ok(MaybeRelocatable::from(Felt252::from(
rel_a.offset as i128 - rel_b.offset as i128,
)));
}
Err(MathError::RelocatableSubDiffIndex(Box::new((
*rel_a, *rel_b,
Expand Down

0 comments on commit 2b7de08

Please sign in to comment.