feat/circom prover removes ark-circom dep. #312
Conversation
KimiWu123
force-pushed
the
feat/circom-prover-remove-ark-circom-dep
branch
from
638fb61 to
3c58dba
Compare
Deploying mopro with
|
| Latest commit: |
9a3b3fd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9a93372d.mopro.pages.dev |
| Branch Preview URL: | https://feat-circom-prover-remove-ar.mopro.pages.dev |
KimiWu123
force-pushed
the
feat/circom-prover-remove-ark-circom-dep
branch
13 times, most recently
from
bb2dfe9 to
66f5726
Compare
KimiWu123
force-pushed
the
feat/circom-prover-remove-ark-circom-dep
branch
from
66f5726 to
3349a3f
Compare
| @@ -39,40 +36,13 @@ pub fn deserialize_inputs<T: Pairing>(data: Vec<u8>) -> SerializableInputs<T> { | |||
| SerializableInputs::deserialize_uncompressed(&mut &data[..]).expect("Deserialization failed") | |||
| } | |||
|
|
|||
| // Convert proof to U256-tuples as expected by the Solidity Groth16 Verifier | |||
There was a problem hiding this comment.
These two functions were moved to mopro-ffi/circom
KimiWu123
force-pushed
the
feat/circom-prover-remove-ark-circom-dep
branch
from
829b44a to
a67411f
Compare
|
@vivianjeng , this PR is ready for review! I decided to keep serialization functions in |
mopro-ffi/src/lib.rs
Outdated
| use circom_prover::{ | ||
| prover::{CircomProof, ProofLib}, | ||
| witness::WitnessFn, | ||
| ProofCalldata, G1, G2, | ||
| }; |
There was a problem hiding this comment.
I think we can remove all these exports?
use circom_prover::{
prover::{CircomProof, ProofLib},
witness::WitnessFn,
ProofCalldata, G1, G2,
};It is not related to the UDL file
There was a problem hiding this comment.
Nice catch, but we still need to keep WitnessFn. It is used in test-e2e/src/lib.rs, like the following,
mopro_ffi::set_circom_circuits! {
("multiplier2_final.zkey", WitnessFn::RustWitness(multiplier2_witness))
}
fixed in 7d42ac1
mopro-ffi/src/lib.rs
Outdated
| ProofCalldata, G1, G2, | ||
| }; | ||
| use mopro_ffi::{GenerateProofResult, MoproError}; | ||
| use circom_prover::witness::WitnessFn; |
There was a problem hiding this comment.
Good point, I didn't think about that. I submitted d3234fb to fix this. The downside of this solution is caller (test-e2e needs to define circom feature in toml file). The other solution is to implement an empty struct like circom_app! and halo2_app!.
There was a problem hiding this comment.
@vivianjeng , I just realized, our current implementation also has this issue. For example, if we disable circom feature, it wll cause an error that set_circom_circuits can't be found. So, I think it's a good idea to address this issue in #306 as you mentioned.
KimiWu123
force-pushed
the
feat/circom-prover-remove-ark-circom-dep
branch
from
d3234fb to
9a3b3fd
Compare
This PR is the second part of #299 , removing
ark-circomdependency.Details:
Copied functions we need from https://github.com/zkmopro/circom-compat/tree/wasm-delete to our repo.