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

Unexpected values using get_integer_from_var_name #1897

Closed
ftheirs opened this issue Dec 17, 2024 · 7 comments
Closed

Unexpected values using get_integer_from_var_name #1897

ftheirs opened this issue Dec 17, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@ftheirs
Copy link

ftheirs commented Dec 17, 2024

Describe the bug
There appears to be an issue in Cairo-vm when accessing certain variables. The behavior and returned values differ depending on whether the variables are defined using let or local.

Check that if you define a with local instead of let, the hint prints the expected value. As far as I checked, in this case where the expression involves a subtraction, the result instead produces the sum of both terms.
If tmp is defined with let, once again, we get the expected result.

Using the custom_hint example, I made a small modification to define a as an expression containing a variable defined with local. This change results in an incorrect output when attempting to read the variable.

func main() {
    alloc_locals;

    local tmp = 3;
    let a = 17 - tmp;
    // Use custom hint to print the value of a
    %{ print(ids.a) %}
    return ();
}

Current output:

20

To Reproduce
Steps to reproduce the behavior:
1 - Clone this branch: https://github.com/Moonsong-Labs/cairo-vm/tree/ft/get_integer_issue
2 - cd examples/custom_hint/
3- make example

Expected output

14
@ftheirs ftheirs added the bug Something isn't working label Dec 17, 2024
@FrancoGiachetta
Copy link
Contributor

Hi there!
I'm taking a look at this issue. Seems like like the problem comes from this operation let a = 17 - tmp. If you change the value of tmp to -3 it will print 14 (when it should be 20 due to the law of signs). Seems like it always behaves as an addition.

@ftheirs
Copy link
Author

ftheirs commented Dec 18, 2024

Hey @FrancoGiachetta! Thanks 💪 The main issue regarding the cairo code is that it belongs to the OS and we cannot change that.
While we were making the upgrade to cairo-lang v0.13.3, we've encountered that behavior in this line:
https://github.com/starkware-libs/cairo-lang/blob/8e11b8cc65ae1d0959328b1b4a40b92df8b58595/src/starkware/starknet/core/os/output.cairo#L78

I think that it must be related to ExpressionSimplifier:
https://github.com/starkware-libs/cairo-lang/blob/8e11b8cc65ae1d0959328b1b4a40b92df8b58595/src/starkware/cairo/lang/compiler/expression_simplifier.py#L62-L65

@FrancoGiachetta
Copy link
Contributor

Oh I see. Thanks a lot for the links. I'll notify you as soon as I have any upgrades.

@FrancoGiachetta
Copy link
Contributor

FrancoGiachetta commented Dec 18, 2024

Reading the comment again, I wanted to clarify something just in case. My first comment is just giving an intuition of the problem, perhaps is not the case. We currently taking a look into it to find a solution.

@FrancoGiachetta
Copy link
Contributor

Hi there!
The issue seems to be coming from the parsing of references, currently they are always being taken as positive values. Due to this and since cairo treats subtractions as additions with one of the operands negated, the operation results in 20.
Do you know other cases like this? It would also help to find other cases with multiplications and divisions.

@FrancoGiachetta
Copy link
Contributor

Hi!
I wanted to let you know that the latest version of main should fix the problem. I'll close this issue, pin us if you need anything else!

@FrancoGiachetta
Copy link
Contributor

FrancoGiachetta commented Jan 6, 2025

I'll close this issue as it was solved in the last main version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants