HungryCatsStudio / circom-compat

Arkworks bindings to Circom's R1CS, for Groth16 Proof and Witness generation in Rust.
Apache License 2.0
1 stars 0 forks source link

Update Wasmer from v2 to v4 #4

Closed jakehemmerle closed 1 week ago

jakehemmerle commented 1 week ago

smt_verifier test passes, but the zkey tests are still failing. I think it makes sense to resolve them in another PR since this is already large enough.

One of the biggest breaking updates from v2 to v4 was the addition of a central Store, which isn't Clone like it was in v2. Resultantly, this Store needed to be shared between both SafeMemory and WasmInstance.

Changing the methods of SafeMemory didn't end up being as hard as I thought, and writing bytes with buffers are still possible, but just on MemoryView instead of Memory. I think there is some inconsistancy in the safety of how memory is read/written, but I can fix that tomorrow.

jakehemmerle commented 1 week ago

@Cesar199999 Concurrency is something I'm still getting comfortable, so I'm happy to learn with you.

Nice job with finding that removing patches solves the zkey tests. I didn't even know patches in the Cargo.toml were a thing. Will remove those for now.

jakehemmerle commented 1 week ago

@mmagician

still wanna take a stab at this or do we leave as-is?

Let's leave it for now; I tried a few different helper refactors, but the main problem is that we can't move store when it's borrowed via view(), even if we return them both to the caller.

Antonio95 commented 1 week ago

Wow, I'm impressed by the depths you've had to dig into! Well done to @jakehemmerle and @Cesar199999 I'm happy to rely on the other two approving PRs