Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bump versions and misc tweaks #51

Merged
merged 1 commit into from
Oct 15, 2023
Merged

Bump versions and misc tweaks #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ark-groth16"
version = "0.3.0"
version = "0.4.0"
authors = [ "arkworks contributors" ]
description = "An implementation of the Groth 2016 zkSNARK proof system"
homepage = "https://arkworks.rs"
Expand Down
25 changes: 16 additions & 9 deletions benches/bench.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ use ark_relations::{
};
use ark_std::ops::Mul;

const NUM_PROVE_REPEATITIONS: usize = 10;
const NUM_VERIFY_REPEATITIONS: usize = 50;
const NUM_PROVE_REPETITIONS: usize = 1;
const NUM_VERIFY_REPETITIONS: usize = 50;
const NUM_CONSTRAINTS: usize = (1 << 20) - 100;
const NUM_VARIABLES: usize = (1 << 20) - 100;

#[derive(Copy)]
struct DummyCircuit<F: PrimeField> {
Expand Down Expand Up @@ -69,22 +71,27 @@ macro_rules! groth16_prove_bench {
let c = DummyCircuit::<$bench_field> {
a: Some(<$bench_field>::rand(rng)),
b: Some(<$bench_field>::rand(rng)),
num_variables: 10,
num_constraints: 65536,
num_variables: NUM_VARIABLES,
num_constraints: NUM_CONSTRAINTS,
};

let (pk, _) = Groth16::<$bench_pairing_engine>::circuit_specific_setup(c, rng).unwrap();

let start = ark_std::time::Instant::now();

for _ in 0..NUM_PROVE_REPEATITIONS {
for _ in 0..NUM_PROVE_REPETITIONS {
let _ = Groth16::<$bench_pairing_engine>::prove(&pk, c.clone(), rng).unwrap();
}

println!(
"per-constraint proving time for {}: {} ns/constraint",
stringify!($bench_pairing_engine),
start.elapsed().as_nanos() / NUM_PROVE_REPEATITIONS as u128 / 65536u128
start.elapsed().as_nanos() / (NUM_PROVE_REPETITIONS as u128 * NUM_CONSTRAINTS as u128)
);
println!(
"wall-clock proving time for {}: {} s",
stringify!($bench_pairing_engine),
start.elapsed().as_secs_f64() / NUM_PROVE_REPETITIONS as f64
);
};
}
Expand All @@ -96,7 +103,7 @@ macro_rules! groth16_verify_bench {
a: Some(<$bench_field>::rand(rng)),
b: Some(<$bench_field>::rand(rng)),
num_variables: 10,
num_constraints: 65536,
num_constraints: NUM_CONSTRAINTS,
};

let (pk, vk) = Groth16::<$bench_pairing_engine>::circuit_specific_setup(c, rng).unwrap();
Expand All @@ -106,14 +113,14 @@ macro_rules! groth16_verify_bench {

let start = ark_std::time::Instant::now();

for _ in 0..NUM_VERIFY_REPEATITIONS {
for _ in 0..NUM_VERIFY_REPETITIONS {
let _ = Groth16::<$bench_pairing_engine>::verify(&vk, &vec![v], &proof).unwrap();
}

println!(
"verifying time for {}: {} ns",
stringify!($bench_pairing_engine),
start.elapsed().as_nanos() / NUM_VERIFY_REPEATITIONS as u128
start.elapsed().as_nanos() / NUM_VERIFY_REPETITIONS as u128
);
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/generator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl<E: Pairing, QAP: R1CSToQAP> Groth16<E, QAP> {
end_timer!(g2_time);

// Compute the B-query in G2
let b_g2_time = start_timer!(|| "Calculate B G2");
let b_g2_time = start_timer!(|| format!("Calculate B G2 of size {}", b.len()));
let b_g2_query = FixedBase::msm::<E::G2>(scalar_bits, g2_window, &g2_table, &b);
drop(g2_table);
end_timer!(b_g2_time);
Expand Down
2 changes: 1 addition & 1 deletion src/prover.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl<E: Pairing, QAP: R1CSToQAP> Groth16<E, QAP> {
let h_assignment = cfg_into_iter!(h)
.map(|s| s.into_bigint())
.collect::<Vec<_>>();
let h_acc = E::G1::msm_bigint(&pk.h_query, &h_assignment);
let h_acc = E::G1::msm_bigint(&pk.h_query, &h_assignment[..h_assignment.len() - 1]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This broke the tests in arkworks-rs/circom-compat#68

@Pratyush I think the reasoning behind this change was that pk.h_query typically has domain - 1 elements right? In which case h_assignment is of length = domain, but the last element is 0.

For some reason, the proving key imported from a .zkey file has length = domain. I'm going to investigate later, but I'm dropping the info here as I go along, maybe you have a clue as for this difference.

cc @jbaylina

Copy link
Member Author

@Pratyush Pratyush Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we would need to change to use msm_unchecked, or something that doesn't check length equality, because the arkworks Groth16 is of length domain - 1. This is because $z_a \cdot z_b - z_c$ is of degree at most 2 * domain - 2, and so $h = (z_a \cdot z_b - z_c)/v_H$ is of degree at most domain - 2 and hence requires domain - 1 coefficients.

Regarding what to replace msm_bigint with, I don't remember if msm_bigint panics when the input lengths are equal or not...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

msm_bigint works fine, it just takes the min length of the two. In arkworks-groth witness calculation this will be domain-1 but in circom, both inputs will be of length domain.

drop(h_assignment);

// Compute C
Expand Down
Loading