-
Notifications
You must be signed in to change notification settings - Fork 36
Bump cairo-vm #386
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
Bump cairo-vm #386
Conversation
HermanObst
left a comment
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.
LGTM
Just some questions to clarify
| /// Convert the base64 encoding of the pie to an unzipped folder. | ||
| pub fn decode_base64_to_unzipped(pie_str: &str, dst: &str) -> Result<(), SnOsError> { | ||
| let buffer = | ||
| general_purpose::STANDARD.decode(pie_str.as_bytes()).map_err(|e| SnOsError::PieZipping(format!("{e}")))?; |
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.
What are these changes? @notlesh
| cairo-lang-casm = { version = "=2.8.2" } | ||
| cairo-type-derive = { version = "0.1.0", path = "crates/cairo-type-derive" } | ||
| cairo-vm = { git = "https://github.com/Moonsong-Labs/cairo-vm", rev = "56b68b50944ecb3123a168218ea7b8b8e23f9be8", features = ["cairo-1-hints", "extensive_hints", "mod_builtin"] } | ||
| cairo-vm = { git = "https://github.com/Moonsong-Labs/cairo-vm", branch = "notlesh/segment-arena-relocation-fix", features = ["cairo-1-hints", "extensive_hints", "mod_builtin"] } |
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.
Did you bump this branch to cairo-vm latest?
@notlesh
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.
Yes, the change here basically does:
- undo the previous hack mentioned in the PR description (by not including the commit 56b68b5)
- include
merge mainwhich pulls in the upstream fix (also mentioned in the PR desc)
|
I'll wait for this PR before raising mine. |
Bump
cairo-vmto latestmain.This also required bumping
cairoto2.8.2which required bumping ourrustcversion.This pulls in an upstream fix which makes this hack obsolete.
Issue Number: N/A
Type
Breaking changes?