WIP: Initial Circom 2 support #10
Conversation
Also print version, n32
- Disable getPtrRawPrime - Write as conditional cfg code blocks
- Add fromArray32 convenience function
field_size in header is 1, not 32 as expected Don't see anything recently changed here: https://github.com/iden3/r1csfile/blob/master/src/r1csfile.js (used by snarkjs) But this seems new: https://github.com/iden3/circom/blob/0149dc0643c3842635fb758efc9ebc102f170a38/constraint_writers/src/r1cs_writer.rs
|
The circom-1 and circom-2 code paths / functions can probably be separated more cleanly, but this works for now. Open to ideas for doing this separation better :) |
src/witness/circom.rs
Outdated
| // Circom 2.0 | ||
| pub fn get_version(&self) -> Result<i32> { | ||
| self.get_i32("getVersion") | ||
| } | ||
|
|
||
| // Circom 1 | ||
| pub fn get_fr_len(&self) -> Result<i32> { | ||
| self.get_i32("getFrLen") | ||
| } | ||
|
|
||
| // Circom 2.0 | ||
| pub fn get_field_num_len32(&self) -> Result<i32> { | ||
| self.get_i32("getFieldNumLen32") | ||
| } |
There was a problem hiding this comment.
You could perhaps create a struct CircomBase (for any shared functions), struct Circom (for 1.0) and struct Circom2 (for 2.0) to better separate 1.0 vs 2.0 functions, and instantiate the struct Wasm with one or the other based on the feature?
There was a problem hiding this comment.
I tried a few different things here, with enums/generics etc. The current trait version is the one that that seemed somewhat sensible and I managed to get to compile 😅 Let me know what you think.
Make normal dependency
Can introduce tracing separately
|
@gakonst Updated PR based on review, please take a look. Any ideas on the R1CSfile header field_size error? |
| use color_eyre::Result; | ||
| use num_bigint::BigInt; | ||
| use num_traits::Zero; | ||
| use std::cell::Cell; | ||
| use wasmer::{imports, Function, Instance, Memory, MemoryType, Module, RuntimeError, Store}; | ||
|
|
||
| use super::{fnv, SafeMemory, Wasm}; | ||
| #[cfg(feature = "circom-2")] |
There was a problem hiding this comment.
This isn't very pretty, but it makes Clippy happy. Is this idiomatic or is there a better way of doing this?
| fn get_ptr_raw_prime(&self) -> Result<i32>; | ||
| } | ||
|
|
||
| pub trait Circom2 { |
There was a problem hiding this comment.
More fns are needed here I think but I suggest it can be merged as experimental WIP support, since it Circom 2 isn't fully functioning yet and it is hidden under a feature flag.
|
Nice work, merged! |
WIP PR towards #8
Minimal subset of WASM interface has been ported based on https://docs.google.com/document/d/1P0j9zXjJRkhnyPT03VjxYBUySC8l4k41ZQ5LY7uyQnw/edit and https://github.com/oskarth/hello-circom/blob/master/multiplier2_js/witness_calculator.js#L74
Can be tested with
cargo test groth16_proof_circom2 --features circom-2. All current tests should pass as normal since the feature flagcircom-2is off by default.Can either clean up and merge as partial progress with minimal WASM interface, or keep open until R1CS format stuff is sorted.
Current state
19faa5c
WIP: Debug R1CSfile header field_size in header is 1, not 32 as expected
Don't see anything recently changed here: https://github.com/iden3/r1csfile/blob/master/src/r1csfile.js (used by snarkjs)
But this seems new: https://github.com/iden3/circom/blob/0149dc0643c3842635fb758efc9ebc102f170a38/constraint_writers/src/r1cs_writer.rs
I also see this: https://github.com/gakonst/ark-circom/blob/11e6d04f3b88a8adb12cc157024ac4b5f11dd74f/src/circom/r1cs_reader.rs#L59-L62 which may or may not be related (haven't investigated in detail and currently not familiar the R1CS file format).