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

[DRAFT] add casm json target #1287

Closed
wants to merge 3 commits into from
Closed

Conversation

tohrnii
Copy link

@tohrnii tohrnii commented Apr 28, 2024

Addressing #676

Copy link
Contributor

@maciektr maciektr left a comment

Choose a reason for hiding this comment

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

Hi!

scarb/src/compiler/compilers/lib.rs Outdated Show resolved Hide resolved

/// Represents a contract in the Starknet network.
#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct SerializedCasm {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

You're right it should ideally go there but I wasn't sure if the cairo team wanted to make this a part of that repo based on some discussion in related issues. I'll open a PR there.

tohrnii and others added 2 commits May 1, 2024 07:04
@maciektr
Copy link
Contributor

maciektr commented May 1, 2024

Now that I think about it, please consider if your usage isn't already covered by a tool called universal-sierra-compiler https://github.com/software-mansion/universal-sierra-compiler/tree/master, which is already capable of compiling Sierra json to Casm json format (and has not been available when the issue has been created).

@tohrnii
Copy link
Author

tohrnii commented May 1, 2024

@maciektr Thank you. That tool should already cover the use case. I'll close this PR.

@tohrnii tohrnii closed this May 1, 2024
@tohrnii
Copy link
Author

tohrnii commented May 1, 2024

@maciektr After discussion with @rodrigo-pino, there are a couple things missing from the generated casm json by the universal-sierra-compiler, namely entrypoint and builtins. It might be better for scarb to produce the json file directly instead of relying on yet another tool since it already generates the sierra json file. What do you think?

@tohrnii tohrnii reopened this May 1, 2024
@Arcticae
Copy link
Member

Arcticae commented May 7, 2024

@tohrnii It can be added to the universal-sierra-compiler if you need it. Happy to accept any PRs there :)

@maciektr
Copy link
Contributor

maciektr commented May 7, 2024

Hi @tohrnii!

Very sorry for the delay in response, we had a national holidays here in Poland, so I've been out of office for a bit.

After discussion with @rodrigo-pino, there are a couple things missing from the generated casm json by the universal-sierra-compiler, namely entrypoint and builtins.

I believe this should be added to USC then. I second @Arcticae here.

It might be better for scarb to produce the json file directly instead of relying on yet another tool since it already generates the sierra json file.

We do see Scarb as primarily Cairo -> Sierra compiler, and the CASM is just an addition here. We recommend using the USC wherever you rely on CASM programatically and the builtin Scarb feature for by-hand debuging. We move our tooling to use USC as well.

Would it be ok for you?

@tohrnii
Copy link
Author

tohrnii commented May 8, 2024

Thank you @maciektr @Arcticae for the confirmation. Sounds good, I'll open a PR there. Closing this one then.

@tohrnii tohrnii closed this May 8, 2024
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.

3 participants