-
Notifications
You must be signed in to change notification settings - Fork 190
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
π§β¨ Store contract abi and source #1124
π§β¨ Store contract abi and source #1124
Conversation
crates/dojo-lang/src/compiler.rs
Outdated
Contract { | ||
name: WORLD_CONTRACT_NAME.into(), | ||
abi: abi.clone(), | ||
source: source.clone(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the sierra? Is it enough to verify a contract + abi with just sierra? I think we want to store the cairo code itself, so we can compile it down to sierra -> casm to verify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, sierra code must be compiled by the verifier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tarrencev I am a bit confused, about what we expect:
- Everything in the manifest
- Everything in a separate file abi + source per contract
- Everything in separate files (1 file for abi + 1 file for the source per contract)
- All the cairo code into a single file (generated by the compiler? I am not aware of that, also it should includes cairo code dependancies to be verifiable) and 1 abi file per contract
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bal7hazar I think the 3rd option would be best, with the flattened source code. I'm not sure exactly how to extract that from the compiler but it should be possible. Ideally we could support the file structure as well, but it might be tricky. Perhaps we handle source in a separate commit and start with abi?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we handle source in a separate commit and start with abi?
@tarrencev, actually I am not sure the abi is required for a verification purpose. If we can have the cairo sources, we should be able to generate abi from it, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bal7hazar yes that is true but abi can still be useful for interacting with the contracts without verification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have overspent time on it, and it is still confused to me.
I think it will be more efficient if someone else knowing better dojo internal flows and rust could work on it
I can prefer to pause this PR, leaving it for anyone wanting to continue or restart from the beginning.
dd36cf5
to
4979471
Compare
6a784ce
to
a08f2cb
Compare
Hey @bal7hazar, do appreciate you time and effort on this one. Thanks again. π |
Closes #1121
I tried to add tests into
metadata_tests.rs
file, but it seems tests in it are not play at all while runningcargo run --bin sozo -- --manifest-path crates/dojo-core/Scarb.toml test
(neither in the CI) not sure where does it come from but I guess it was the case before I did anything, so probably expected for some reason.I decided to override default WorldMetadata (with
abi
andsource
) by the user manifest, so I guess both fields could be overridden by a user input, which is more flexible but mb less secure.About this topic, I understood to add the sierra program code into the manifest file generated after a build, I hope I have git it correctly.
Workload ~ 5h