lambdaclass / cairo-vm

cairo-vm is a Rust implementation of the Cairo VM. Cairo (CPU Algebraic Intermediate Representation) is a programming language for writing provable programs, where one party can prove to another that a certain computation was executed correctly without the need for this party to re-execute the same program.
https://lambdaclass.github.io/cairo-vm
Apache License 2.0
485 stars 132 forks source link

Fix WASM32 compilation for Cairo1-run and cairo-vm #1792

Open wraitii opened 1 week ago

wraitii commented 1 week ago

Fix WASM32 compilation for Cairo1-run and cairo-vm

Description

Running the following fails on main: cargo build --bin cairo1-run --target wasm32-unknown-unknown --release --no-default-features

There are 4 issues:

I also remove a comment to check WASM compatibility with some libraries, as they are indeed compatible.

This PR fixes both cairo1-run and cairo-vm for WASM targets.

Oppen commented 1 week ago
  • Two lines in vm/src/lib.rs and vm/src/vm/runners/cairo_pie.rs pessimistically assume wasm <=> no_std, when the former works just fine - we can be more precise.

Those really shouldn't be handled there in the first place. We have the stdlib module to abstract those differences away.

wraitii commented 1 week ago
  • Two lines in vm/src/lib.rs and vm/src/vm/runners/cairo_pie.rs pessimistically assume wasm <=> no_std, when the former works just fine - we can be more precise.

Those really shouldn't be handled there in the first place. We have the stdlib module to abstract those differences away.

I'm not sure if this means I need to update the PR or if it's a general comment, could you clarify ?


Should I add this as a fix to the changelog? The PR template wasn't clear on this (this isn't a change that needs to be 'documented', per se).

Oppen commented 1 week ago

I'm not sure if this means I need to update the PR or if it's a general comment, could you clarify ?

Just a general comment. Your fix for those is OK. We should clean up at some later point.

Oppen commented 1 week ago

Should I add this as a fix to the changelog? The PR template wasn't clear on this (this isn't a change that needs to be 'documented', per se).

Just the gist, something like "compilation of cairo-1-run and cairo-vm on wasm32 has been fixed" if the latest release was already broken. If only main broke and you fixed it before it reached a release, then it shouldn't affect users reading the changelog.

The intention of the template (we may need to rephrase it) is to document functional changes mostly. We consider non-functional changes thing like refactors, cleanups, or improvements to the build system or the CI, same for simply adding tests, while bug fixes for reported issues, significant performance changes or added/broken interfaces need to be in the changelog.

Oppen commented 1 week ago

Oh BTW, congrats! 🎉 I just noticed it's your first PR. It's on the right track for merging.

Oppen commented 1 week ago

Also, I played a bit of 0AD like 10 years ago or so. I didn't do much because IIRC there were no campaigns I had nobody to play with. Nice to cross paths with a contributor.

wraitii commented 1 week ago

Just the gist, [...]

Added a line, can't hurt much anyways.

Also, I played a bit of 0AD like 10 years ago or so. I didn't do much because IIRC there were no campaigns I had nobody to play with. Nice to cross paths with a contributor.

Ah that's fun 😄 we have support for campaigns nowadays, but IIRC still none of them. Though a small multiplayer user base exists.