-
Notifications
You must be signed in to change notification settings - Fork 186
Identifiers for aliases are not built properly #2041
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
Comments
Hi @enitrat! {
"prime": "0x800000000000011000000000000000000000000000000000000000000000001",
"attributes": [],
"debug_info": {
"instruction_locations": {}
},
"data": [
],
"builtins": [],
"hints": {},
"reference_manager": {
"references": []
},
"identifiers": {
"__main__.main": {
"decorators": [],
"pc": 0,
"type": "function"
},
"__main__.compare_abs_arrays.SIZEOF_LOCALS": {
"type": "const",
"value": -3618502788666131213697322783095070105623107215331596699973092056135872020481
},
"starkware.cairo.common.cairo_keccak.keccak.unsigned_div_rem": {
"destination": "starkware.cairo.common.math.unsigned_div_rem",
"type": "alias"
},
"starkware.cairo.common.cairo_keccak.packed_keccak.ALL_ONES": {
"type": "const",
"value": -106710729501573572985208420194530329073740042555888586719234
},
"starkware.cairo.common.cairo_keccak.packed_keccak.BLOCK_SIZE": {
"type": "const",
"value": 3
},
"starkware.cairo.common.alloc.alloc.SIZEOF_LOCALS": {
"type": "const",
"value": 0
},
"starkware.cairo.common.uint256.SHIFT": {
"type": "const",
"value": 340282366920938463463374607431768211456
}
}
} As you can see, |
Hey @FrancoGiachetta, please TAL at #2042 which partially solves the issue by making it possible to resolve the original path for aliases. I'll get back to you tomorrow for more in-depth discussions! Thanks
Indeed, it looks like the compiled program doesn't have information about this. I was a bit disturbed because even though the program doesn't have this info, we should have a way to resolve the aliases anyway: hence my idea of adding the "destination" field in the Identifier model, which can trivially be done by Serde. |
I see, I'll it then! |
Hey @enitrat, I believe this issue has been resolved, right? I'll write a summary on this issue: SummaryThere was not enough information to resolve aliases from within a hint:
Now, If we have the following import
And the following hint:
Then as long as we have access to the identifiers from within the hint processor, we can:
|
Yes perfect! |
TLDR: I think the Identifiers for aliases are missing some useful data; example: in this test I would expect the identifier to have the full_path set to to the "destination" value - or something similar
cairo-vm/vm/src/serde/deserialize_program.rs
Lines 1035 to 1043 in d101ee8
I'm working on making all python hints compatible to use with the RustVM, and for that, i'm currently working on making the constant of the programs resolvable and accessible through an
ids
object.For that I need to properly retrieve the values associated to constants.
Here are my tests (done in https://github.com/kkrt-labs/keth repo & hard to repro as i'm still fiddling around):
Test 1: test with a locally defined constant. This works. If I log some information, I can see that at some point, I'm doing:
Test 2: verifying that non-important program constants are not accssible. works fine
Test 3: verifying if imported program constants are accessible. This however is problematic: here's what searching the
Identifiers
yields:As you can see this is correctly being retrieved as an Identifier of type
alias
which is expected. However there's no associated value, nor any other type information.I don't know if this is intended or not? I would expect there to be more info, perhaps,
full_name
resolving to the path of the constant in the program? This way I could easily use theconstants
argument passed in all hints execution to get the proper value.This is happening during the
execute_hint
step, relevant code that I wrote:Second note: I made of fork of the CairoVM to have
hint_accessible_scopes
available during hint executions. Which I believe should be there. I will submit a PR isolated for this.For reference, in the python VM, I can get the original path from the alias.
The text was updated successfully, but these errors were encountered: