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

bug: Unknown Hint: Core(Core(EvalCircuit #366

Open
notlesh opened this issue Sep 11, 2024 · 3 comments
Open

bug: Unknown Hint: Core(Core(EvalCircuit #366

notlesh opened this issue Sep 11, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@notlesh
Copy link
Collaborator

notlesh commented Sep 11, 2024

Current behavior:

Hint fails with:

Unknown Hint: Core(Core(EvalCircuit { n_add_mods: Deref(CellRef { register: AP, offset: -6 }), add_mod_builtin: Deref(CellRef { register: FP, offset: -6 }), n_mul_mods: Deref(CellRef { register: AP, offset: -4 }), mul_mod_builtin: Deref(CellRef { register: FP, offset: -5 }) }))

This happens presumably any time mul_mod is called. The problem is that this is not handled in cairo-vm.

Expected behavior:

Hint should succeed.

Steps to reproduce:

Run prove_block against affected blocks such as 160035, 160045, 160068, 160074, 160084, or 160093

@notlesh notlesh added the bug Something isn't working label Sep 11, 2024
@notlesh notlesh self-assigned this Sep 11, 2024
@notlesh
Copy link
Collaborator Author

notlesh commented Sep 11, 2024

I started by handling the hint in cairo-vm: Moonsong-Labs/cairo-vm#44

But this led to an error shortly later: Unknown memory cell at address 13:6

It turns out there is some logic missing from cairo-vm's fill_memory as compared to cairo-lang's. This comment (although unclear no me) suggests what is wrong:

# Note that we can't read 'n' here because sierra expects this function to compute it.

https://github.com/starkware-libs/cairo-lang/blob/4ea4fe8e167845a3402ae2ea0a8b6004aad18dd5/src/starkware/cairo/lang/builtins/modulo/mod_builtin_runner.py#L373

@notlesh
Copy link
Collaborator Author

notlesh commented Sep 11, 2024

A naive attempt to avoid reading n, as suggested in the comment mentioned above, isn't sufficient. It turns out there is a lot of extra code to calculate this value instead of reading it.

I began implementing this, but it turned into quite a rabbit hole, as there seems to be a lot more code omitted in the cairo-vm implementation. Even worse, a lot of the code that is omitted is actually dead code, so it's not obvious what exactly actually needed among the missing code.

@notlesh
Copy link
Collaborator Author

notlesh commented Sep 11, 2024

The implementation from cairo seems to work 🎉 We're now passing the following blocks:

160035, 160045, 160068, 160074, 160084

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

1 participant