arkworks-rs / groth16 Goto Github PK

Summary of Bug

Facing issues while serializing and deserializing the proof. The proof is valid and satisfies all the constraints.

let proof: Proof<Bls12_381> = prove_groth16(&pk, circuit.clone(), &mut rng);
let mut proof_bytes = vec![0u8; proof.serialized_size()];
proof.serialize(&mut proof_bytes).unwrap();
let proof_anew = Proof::<Bls12_381>::deserialize(&proof_bytes[..]).unwrap();

The error I'm getting is SerializationError::InvalidData on line: https://github.com/webb-tools/arkworks-gadgets/blob/bug-reprodution/src/setup/mixer.rs#L625

If I call Proof::deserialize_unchecked instead of Proof::deserialize I'm getting a SerializationError:: UnexpectedFlags

Version

ark-groth16 = {version = "^0.2.0", default-features = false }

Steps to Reproduce

Clone this repo: https://github.com/webb-tools/arkworks-gadgets
Checkout to bug-reprodution branch
Run cargo test should_handle_proof_deserialization --features r1cs
Which will run this test: https://github.com/webb-tools/arkworks-gadgets/blob/bug-reprodution/src/setup/mixer.rs#L607-L639

If it is ,Any codes about that ?

Summary

We should support verifier SRS points for public inputs being independent of the circuit and other SRS elements, but maybe only optionally so. ZEXE-like schemes could then be build using MIPP proofs like SnarkPack, without wasting resources on slow recursion.

Proposal

At present, we discover verifier SRS points for public inputs only after performing a Groth16 trusted setup because Groth16 constructs verifier SRS points and prover SRS points for C similarly.

As an optional tweak, I'd propose that verifier SRS public input points should be at least partially constructed via hash-to-curve before running the SRS. We then add one additional prover SRS point for C for each public input point like this, which expresses the actual wiring and binds the hashed-to-curve public input points into the circuit.

We could then construct arbitrary circuits in the future which have (some of) the same public inputs as older circuits, which then simplifies using diverse Groth16 circuits within another proof system like SnarkPack.

We likely fix gamma=1 when doing this, but then each new circuit has its own fresh delta, making [delta]_2 possibly the unique place where SnarkPack proves correctness of the Groth16 circuits used. It's likely the groth16 crate only needs support for circuit Independence of public input SRS points though, and anything else can happen in other crates.

I'll likely convince someone in-house to work on this, but we'll want to upstream the changes, and this issue might spark useful conversation.

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

We can save 1 FFT (of 7) if we compute the h_query points in the evaluation form during the setup. We can similarly avoid the division when computing h by normalizing these points by g^n - 1 (evaluations of the vanishing polynomial on the coset).
See the PoC here: https://github.com/achimcc/groth16/pull/1/files

These changes break existing proving keys, though I believe they can be updated. Does it worth the burden?

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

This issue was run into on the zexe library and has not been attempted to be reproduced on arkworks yet.

The constraints method verify for computing a BooleanVar representing whether a proof is valid or not produces unsatisfied constraints when a Default::default proof is passed as input. The correct behavior would be to simply return a BooleanVar with value false.

The method works as expected when passing in the correct proof (returns a true BooleanVar) or passing in a proof with values not initialized to Default, e.g. random group elements or canonical generator (returns a false BooleanVar). This indicates that there might be some odd behavior with how verify interacts with a proof that has 0 values (which I believe is what Default sets the group elements to).

https://github.com/arkworks-rs/groth16/blob/master/src/constraints.rs#L243

it support bn128 or bn256 curve?

Summary

We should likely add batch verification like ZCash deployed eventually, and perhaps snarkpack.

Proposal

The ZCash issue zcash/librustzcash#253 (comment) has inbound links with further details.

In this vein, Arkworks might have or want other relevant abstractions, like MIPP, which maybe simplify SnarkPack, which requires more effort and perhaps another crate.

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

@npwardberkeley @weikengchen Do you have implementations of the SNARK trait for Groth16? Also, is there a corresponding gadget trait somewhere? We should put that trait in ark-crypto-primitives.

Hi, I want to know is Circom supported now ? If not will it be possible one day? And when will it be

This issue describes the branch commit-carrying.

According to LegoSNARK, Groth16 is a commit-carrying SNARK with weak binding. With a small modification below, Groth16 can become a full-fledged commit-carrying SNARK. Note that Groth16 is commit-carrying, but not commit-and-prove because the commitment's parameters are relation-dependent.

The modification is as follows:

• Augment the QAP so that the first n columns of the A matrices have a rank at least n, where n is the number of input elements. This step is done by simply adding n+1 constraints. A discussion is here: https://github.com/arkworks-rs/groth16/blob/commit-carrying/src/augmented_qap.rs#L1
• Modify the data structures of proving keys and verifying keys accordingly.
• Modify the proof generation and verification accordingly.

This issue is, very likely, not going to be merged into the master branch since its interface is drastically different.

More updates will be added to this note.

I measured the cost for Groth16 to prove a constraint system with two million constraints, with a different number of cores, using cargo bench in this repo with appropriate command line parameters.

Below I focus on the main cost in my constraint system, which turns out to be in the witness map and computation of C.

Going from 1 core to 4 cores, the improvement is significant. But later when more cores are added, the time for the witness map seems to not change a lot.

Since the witness map involves a lot of FFT, it may suggest that the current implementation of FFT has a barrier toward many-many-core parallelism.

Such a barrier, maybe avoidable, maybe unavoidable. I will take a look at the detailed breakdown of the cost of the witness map.

===== Groth16 =====

1 core
R1CS to QAP witness map ... 32.102s
Compute C ................. 56.453s
Total ..................... 93.780s

4 cores
R1CS to QAP witness map ... 12.476s
Compute C ................. 16.54s
Total ..................... 34.203s

20 cores
R1CS to QAP witness map ... 8.856s
Compute C ................. 8.631s
Total ..................... 23.99s

40 cores
R1CS to QAP witness map ... 8.300s
Compute C ................. 4.414s
Total ..................... 18.319s

48 cores
R1CS to QAP witness map ... 8.166s
Compute C ................. 4.682s
Total ..................... 18.230s

Using cargo to build the library with command
cargo build --release

Current cargo/rustc version 1.75.0.

Then, get the following error:

error[E0432]: unresolved import `ark_ec::scalar_mul::fixed_base`
 --> src/generator.rs:2:44
  |
2 | use ark_ec::{pairing::Pairing, scalar_mul::fixed_base::FixedBase, CurveGroup};
  |                                            ^^^^^^^^^^ could not find `fixed_base` in `scalar_mul`

For more information about this error, try `rustc --explain E0432`.
error: could not compile `ark-groth16` (lib) due to previous error```

Summary of Bug

Apologies if this is not the correct repo for this.

I'm proving circom circuits on the bls12-381 curve using ark-groth16. I've run into a bug where ark-groth16 generates invalid proofs for zkeys that are built against powers of tau ceremonies that are not at least twice as big.

e.g. i have a circuit with 2500 constraints. I build the zkey using a ptau with 2^12 powers. snarkjs generates valid proofs, ark-groth16 generates invalid proofs. I build the same circuit against a ptau with 2^13 powers and both snarkjs and ark-groth16 generate valid proofs.

I've confirmed that the proofs generated by ark-groth16 are invalid in snarkjs as well. This bug does not exist for the alt_bn128 curve.

I'm using the R1CS QAP reduction in ark-circom.

I know there's lots of places where the inconsistency could come from. I wanted to post here to see if anyone has ideas off the top of their head.

Version

[email protected]

Steps to Reproduce

Build a circuit over the bls12-381 curve with N constraints and < 2*N ptau powers. Use the R1CS QAP mapping in ark-circom to generate a proof. Try to validate the proof.

This is implemented here, the relevant zkeys/witness logic is committed in the same repo. I'll break this into a minimal repro example if there's any interest.

I recently read the DIZK codebase https://github.com/scipr-lab/dizk, where I did not find the proving process contains things like inline_all_lcs(). Previously I found that the inline_all_lcs() is serially executed and could take up a lot of time in the total proving time. I think that the inline_all_lcs()'s worst-case time complexity could be O(n^2) where n is the number of linear combinations, and this really hurts large circuit performance.

https://github.com/arkworks-rs/snark/blob/e0e29bfcbd88ba703aec58d8561db894f602439d/relations/src/r1cs/constraint_system.rs#L309:12