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: Add lessThanOrEqualAddress cairo hint #132

Conversation

renzobanegass
Copy link
Contributor

Closes #122

This pull request implements the TestLessThanOrEqualAddress hint as described in the issue. The hint checks whether the value at lhs (a relocatable address) is less than or equal to the value at rhs (another relocatable address) and stores the boolean result (0 or 1) at dst. I'll leave it as a draft for now as I didn't create the cairo integration test yet, I would like some help with that because I don't get how those programs work RN. And would like to get feedback over the implementation and the tests.

Changes Made

  • Added the TestLessThanOrEqualAddress hint in hints/math/testLessThanOrEqualAddress.ts and it's Parser.
  • Registered the new hint in hints/hintProcessor.ts and hints/hintHandler.ts
  • Added unit tests to verify the parsing and logic of the TestLessThanOrEqualAddress hint.

Copy link
Collaborator

@zmalatrax zmalatrax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great :)

src/hints/math/testLessThanOrEqualAddress.ts Outdated Show resolved Hide resolved
src/hints/hintHandler.ts Outdated Show resolved Hide resolved
src/hints/math/testLessThanOrEqualAddress.ts Outdated Show resolved Hide resolved
src/hints/math/testLessThanOrEqualAddress.ts Outdated Show resolved Hide resolved
src/hints/math/testLessThanOrEqualAddress.ts Outdated Show resolved Hide resolved
src/hints/math/testLessThanOrEqualAddress.test.ts Outdated Show resolved Hide resolved
@zmalatrax
Copy link
Collaborator

@renzobanegass For the integration test, you have to write a Cairo program in the cairo_programs/cairo/hints folder that makes use of the TestLessThanOrEqualAddress.

This hint is used by the multi_pop_front() and multi_pop_back() methods of corelib array.cairo.
So you should make a Cairo program using such methods

A simple program such as

fn main() {
    let mut span = array![1, 2, 3].span();
    let _ = span.multi_pop_back::<2>();
}

@renzobanegass
Copy link
Contributor Author

@renzobanegass For the integration test, you have to write a Cairo program in the cairo_programs/cairo/hints folder that makes use of the TestLessThanOrEqualAddress.

This hint is used by the multi_pop_front() and multi_pop_back() methods of corelib array.cairo. So you should make a Cairo program using such methods

A simple program such as

fn main() {
    let mut span = array![1, 2, 3].span();
    let _ = span.multi_pop_back::<2>();
}

When trying to compile cairo I got this error:
image

@renzobanegass
Copy link
Contributor Author

I pushed some changes, I couldn't compile the cairo programs because of the error I showed you before, I added two, one for the multi_pop_front() and other for the multi_pop_back()

The unit tests are working.

@zmalatrax
Copy link
Collaborator

When trying to compile cairo I got this error:

It looks like you haven't the Cairo Zero toolchain (cairo-lang 0.13.1) installed in your current python environment.

Poetry facilitates the management of dependencies, you should have Poetry installed, installation guide here

Then in the root of the project set a poetry env to use python3.10: poetry use python3.10
And install the dependencies of the project (i.e. cairo-lang): poetry install

Then you should be able to compile Cairo Zero files.

@zmalatrax
Copy link
Collaborator

⚠️ Your commits are not signed, please configure a GPG key to sign your commits

Also, your typescript files are not formatted as expected. We're using Trunk, install the IDE extension if you're using VSCode or Neovim, otherwise install their CLI and run trunk fmt

@renzobanegass

This comment was marked as outdated.

@renzobanegass renzobanegass force-pushed the feat/add-testlessthanorequaladdress-hint branch from 17227a8 to a7aca56 Compare August 8, 2024 17:42
@renzobanegass
Copy link
Contributor Author

I solved it, don't worry. I'll check the programs are working.

@renzobanegass
Copy link
Contributor Author

When running any programs I get this error
image

@zmalatrax
Copy link
Collaborator

zmalatrax commented Aug 8, 2024

When running any programs I get this error

You're using the cairo-lang command cairo-run which is made for Cairo Zero programs only.
cairo-lang is only used for compilation purposes and diff-testing against other Cairo VM implementations.

You should use the the Cairo VM TS CLI command, check the README

You can do make build and then you'll have the Cairo VM TS CLI commands available.
The target make compile will compile all the available Cairo and Cairo Zero programs.
Check the availables options with cairo --help or cairo run --help

For your program, something like this would execute it: cairo run --layout recursive cairo_programs/cairo/hints/test_less_than_or_equal_address.json

@renzobanegass
Copy link
Contributor Author

renzobanegass commented Aug 8, 2024

Thanks! I missed that part completely.
image

@renzobanegass renzobanegass marked this pull request as ready for review August 8, 2024 18:55
Copy link
Collaborator

@zmalatrax zmalatrax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, just some details on the JSDoc :)

src/hints/math/testLessThanOrEqualAddress.ts Outdated Show resolved Hide resolved
src/hints/math/testLessThanOrEqualAddress.ts Outdated Show resolved Hide resolved
@renzobanegass
Copy link
Contributor Author

All done!

@renzobanegass renzobanegass force-pushed the feat/add-testlessthanorequaladdress-hint branch from 4db92c6 to c2fd248 Compare October 7, 2024 14:10
Copy link
Collaborator

@zmalatrax zmalatrax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good

@zmalatrax zmalatrax merged commit be4c9d0 into kkrt-labs:main Oct 7, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: add TestLessThanOrEqualAddress
2 participants