Arkworks adapter for Groth16 Backend #701
Conversation
irfanbozkurt
commented
Nov 28, 2023
•
irfanbozkurt
commented
- Adds an adapter converting arkworks circuits (bls12-381) to lambdaworks QAP.
- Adds tests that proves arkworks circuits using lambdaworks groth16 backend
- Solves a small bug in QAP implementation (number of gates function)
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #701 +/- ##
==========================================
+ Coverage 95.41% 95.44% +0.02%
==========================================
Files 137 139 +2
Lines 31164 31367 +203
==========================================
+ Hits 29735 29938 +203
Misses 1429 1429 ☔ View full report in Codecov by Sentry. |
irfanbozkurt
changed the title
|
You have conflicts, but it can be merged |
| let borrowed_cs_ref = binding.deref(); | ||
|
|
||
| let ark_witness = &borrowed_cs_ref.witness_assignment; | ||
| let ark_io = &borrowed_cs_ref.instance_assignment[1..].to_vec(); |
There was a problem hiding this comment.
Comment why is it 1, so it's not a magical number
There was a problem hiding this comment.
It was unnecessary. Removed.
provers/groth16/src/qap.rs
Outdated
| assert_eq!(num_of_total_inputs, r.len()); | ||
| assert_eq!(num_of_total_inputs, o.len()); | ||
| assert!(num_of_total_inputs > 0); | ||
| assert!(num_of_public_inputs <= num_of_total_inputs); |
There was a problem hiding this comment.
This can panic, it should return an Error
There was a problem hiding this comment.
This method is only for testing purposes. I moved the method to an utils.rs file within the tests.
There was a problem hiding this comment.
If that's the case you can add the conditional compile flag, the cfg tests. Or just move it to the test module
provers/groth16/src/qap.rs
Outdated
| r, | ||
| o, | ||
| num_of_gates: next_power_of_two, | ||
| num_of_public_inputs: r1cs.number_of_inputs + 1, // Compansate for "1" |
There was a problem hiding this comment.
Typo, compansate. Also, it doesn't say why so it's not quite useful
There was a problem hiding this comment.
Unnecessary 1. My mistake. Removed.
| pub fn to_lambda<F: PrimeField>( | ||
| cs: &ConstraintSystemRef<F>, | ||
| ) -> (QuadraticArithmeticProgram, Vec<FrElement>) { | ||
| ( | ||
| QuadraticArithmeticProgram::from_r1cs(r1cs_from_arkworks_cs(cs)), | ||
| extract_witness_from_arkworks_cs(cs), | ||
| ) | ||
| } |
There was a problem hiding this comment.
ark_cs_to_lambda_cs would be a better that name
Also I'd document with /// what this is doing, the QAP is clear, but you need to check the code to realize the second parameter is the witness
provers/groth16/src/qap.rs
Outdated
| assert_eq!(num_of_total_inputs, r.len()); | ||
| assert_eq!(num_of_total_inputs, o.len()); | ||
| assert!(num_of_total_inputs > 0); | ||
| assert!(num_of_public_inputs <= num_of_total_inputs); |
There was a problem hiding this comment.
If that's the case you can add the conditional compile flag, the cfg tests. Or just move it to the test module
irfanbozkurt
dismissed stale reviews from MauroToscano and diegokingston
via
001a775
