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.

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

Deterministic tests #349

Merged
merged 9 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ members = [
"halo2_frontend",
"halo2_middleware",
"halo2_backend",
"halo2_debug",
"p3_frontend",
]
resolver = "2"
4 changes: 2 additions & 2 deletions halo2_backend/src/poly/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub trait CommitmentScheme {
type ParamsVerifier: for<'params> ParamsVerifier<'params, Self::Curve>;

/// Wrapper for parameter generator
fn new_params(k: u32) -> Self::ParamsProver;
fn new_params(k: u32, rng: impl RngCore) -> Self::ParamsProver;

/// Wrapper for parameter reader
fn read_params<R: io::Read>(reader: &mut R) -> io::Result<Self::ParamsProver>;
Expand Down Expand Up @@ -69,7 +69,7 @@ pub trait Params<C: CurveAffine>: Sized + Clone + Debug {
/// Parameters for circuit synthesis and prover parameters.
pub trait ParamsProver<C: CurveAffine>: Params<C> {
/// Returns new instance of parameters
fn new(k: u32) -> Self;
fn new(k: u32, rng: impl RngCore) -> Self;

/// This computes a commitment to a polynomial described by the provided
/// slice of coefficients. The commitment may be blinded by the blinding
Expand Down
13 changes: 7 additions & 6 deletions halo2_backend/src/poly/ipa/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::poly::{Coeff, LagrangeCoeff, Polynomial};

use group::{Curve, Group};
use halo2_middleware::zal::traits::MsmAccel;
use rand_core::RngCore;
use std::marker::PhantomData;

mod prover;
Expand Down Expand Up @@ -45,8 +46,8 @@ impl<C: CurveAffine> CommitmentScheme for IPACommitmentScheme<C> {
type ParamsProver = ParamsIPA<C>;
type ParamsVerifier = ParamsVerifierIPA<C>;

fn new_params(k: u32) -> Self::ParamsProver {
ParamsIPA::new(k)
fn new_params(k: u32, rng: impl RngCore) -> Self::ParamsProver {
ParamsIPA::new(k, rng)
}

fn read_params<R: io::Read>(reader: &mut R) -> io::Result<Self::ParamsProver> {
Expand Down Expand Up @@ -150,7 +151,7 @@ impl<C: CurveAffine> Params<C> for ParamsIPA<C> {
impl<C: CurveAffine> ParamsProver<C> for ParamsIPA<C> {
/// Initializes parameters for the curve, given a random oracle to draw
/// points from.
fn new(k: u32) -> Self {
fn new(k: u32, _: impl RngCore) -> Self {
adria0 marked this conversation as resolved.
Show resolved Hide resolved
// This is usually a limitation on the curve, but we also want 32-bit
// architectures to be supported.
assert!(k < 32);
Expand Down Expand Up @@ -253,7 +254,7 @@ mod test {
use halo2curves::pasta::{EpAffine, Fq};

let engine = H2cEngine::new();
let params = ParamsIPA::<EpAffine>::new(K);
let params = ParamsIPA::<EpAffine>::new(K, OsRng);
let domain = EvaluationDomain::new(1, K);

let mut a = domain.empty_lagrange();
Expand Down Expand Up @@ -282,7 +283,7 @@ mod test {
use halo2curves::pasta::{EqAffine, Fp};

let engine = H2cEngine::new();
let params: ParamsIPA<EqAffine> = ParamsIPA::<EqAffine>::new(K);
let params: ParamsIPA<EqAffine> = ParamsIPA::<EqAffine>::new(K, OsRng);
let domain = EvaluationDomain::new(1, K);

let mut a = domain.empty_lagrange();
Expand Down Expand Up @@ -322,7 +323,7 @@ mod test {
let rng = OsRng;

let engine = H2cEngine::new();
let params = ParamsIPA::<EpAffine>::new(K);
let params = ParamsIPA::<EpAffine>::new(K, OsRng);
let mut params_buffer = vec![];
<ParamsIPA<_> as Params<_>>::write(&params, &mut params_buffer).unwrap();
let params: ParamsIPA<EpAffine> = Params::read::<_>(&mut &params_buffer[..]).unwrap();
Expand Down
3 changes: 2 additions & 1 deletion halo2_backend/src/poly/ipa/msm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,14 +227,15 @@ mod tests {
pasta::{Ep, EpAffine, Fp, Fq},
CurveAffine,
};
use rand_core::OsRng;

#[test]
fn msm_arithmetic() {
let base: Ep = EpAffine::from_xy(-Fp::one(), Fp::from(2)).unwrap().into();
let base_viol = base + base;

let engine = H2cEngine::new();
let params = ParamsIPA::new(4);
let params = ParamsIPA::new(4, OsRng);
let mut a: MSMIPA<EpAffine> = MSMIPA::new(&params);
a.append_term(Fq::one(), base);
// a = [1] P
Expand Down
15 changes: 8 additions & 7 deletions halo2_backend/src/poly/kzg/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use halo2_middleware::ff::{Field, PrimeField};
use halo2_middleware::zal::traits::MsmAccel;
use halo2curves::pairing::Engine;
use halo2curves::{CurveAffine, CurveExt};
use rand_core::{OsRng, RngCore};
use rand_core::RngCore;
use std::fmt::Debug;
use std::marker::PhantomData;

Expand Down Expand Up @@ -139,8 +139,8 @@ where
type ParamsProver = ParamsKZG<E>;
type ParamsVerifier = ParamsVerifierKZG<E>;

fn new_params(k: u32) -> Self::ParamsProver {
ParamsKZG::new(k)
fn new_params(k: u32, rng: impl RngCore) -> Self::ParamsProver {
ParamsKZG::new(k, rng)
}

fn read_params<R: io::Read>(reader: &mut R) -> io::Result<Self::ParamsProver> {
Expand Down Expand Up @@ -429,8 +429,8 @@ where
E::G1: CurveExt<AffineExt = E::G1Affine>,
E::G2Affine: SerdeCurveAffine,
{
fn new(k: u32) -> Self {
Self::setup(k, OsRng)
fn new(k: u32, rng: impl RngCore) -> Self {
Self::setup(k, rng)
}

fn commit(
Expand All @@ -455,6 +455,7 @@ mod test {
use crate::poly::kzg::commitment::ParamsKZG;
use halo2_middleware::ff::Field;
use halo2_middleware::zal::impls::H2cEngine;
use rand_core::OsRng;

#[test]
fn test_commit_lagrange() {
Expand All @@ -466,7 +467,7 @@ mod test {
use halo2curves::bn256::{Bn256, Fr};

let engine = H2cEngine::new();
let params = ParamsKZG::<Bn256>::new(K);
let params = ParamsKZG::<Bn256>::new(K, OsRng);
let domain = EvaluationDomain::new(1, K);

let mut a = domain.empty_lagrange();
Expand All @@ -492,7 +493,7 @@ mod test {
use super::super::commitment::Params;
use halo2curves::bn256::Bn256;

let params0 = ParamsKZG::<Bn256>::new(K);
let params0 = ParamsKZG::<Bn256>::new(K, OsRng);
let mut data = vec![];
<ParamsKZG<_> as Params<_>>::write(&params0, &mut data).unwrap();
let params1: ParamsKZG<Bn256> = Params::read::<_>(&mut &data[..]).unwrap();
Expand Down
8 changes: 4 additions & 4 deletions halo2_backend/src/poly/multiopen_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ mod test {
const K: u32 = 4;

let engine = H2cEngine::new();
let params = ParamsIPA::<EqAffine>::new(K);
let params = ParamsIPA::<EqAffine>::new(K, OsRng);

let proof = create_proof::<
IPACommitmentScheme<EqAffine>,
Expand Down Expand Up @@ -67,7 +67,7 @@ mod test {
const K: u32 = 4;

let engine = H2cEngine::new();
let params = ParamsIPA::<EqAffine>::new(K);
let params = ParamsIPA::<EqAffine>::new(K, OsRng);

let proof = create_proof::<
IPACommitmentScheme<EqAffine>,
Expand Down Expand Up @@ -105,7 +105,7 @@ mod test {
const K: u32 = 4;

let engine = H2cEngine::new();
let params = ParamsKZG::<Bn256>::new(K);
let params = ParamsKZG::<Bn256>::new(K, OsRng);

let proof = create_proof::<_, ProverGWC<_>, _, Blake2bWrite<_, _, Challenge255<_>>>(
&engine, &params,
Expand Down Expand Up @@ -138,7 +138,7 @@ mod test {
const K: u32 = 4;

let engine = H2cEngine::new();
let params = ParamsKZG::<Bn256>::new(K);
let params = ParamsKZG::<Bn256>::new(K, OsRng);

let proof = create_proof::<
KZGCommitmentScheme<Bn256>,
Expand Down
26 changes: 26 additions & 0 deletions halo2_debug/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
[package]
name = "halo2_debug"
version = "0.3.0"
authors = [
"Privacy Scaling Explorations team",
]
edition = "2021"
rust-version = "1.66.0"
description = """
Halo2 Debug. This package contains utilities for debugging and testing within
the halo2 ecosystem.
"""
license = "MIT OR Apache-2.0"
repository = "https://github.com/privacy-scaling-explorations/halo2"
documentation = "https://privacy-scaling-explorations.github.io/halo2/"
categories = ["cryptography"]
keywords = ["halo", "proofs", "zkp", "zkSNARKs"]

[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs", "--html-in-header", "katex-header.html"]

[dependencies]
tiny-keccak = { version = "2.0.2", features=["keccak"] }
hex = "0.4.3"
rand_core = "0.6.4"
30 changes: 30 additions & 0 deletions halo2_debug/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use rand_core::block::BlockRng;
use rand_core::block::BlockRngCore;
use tiny_keccak::Hasher;

/// One number generator, that can be used as a deterministic Rng, outputing fixed values.
pub struct OneNg {}

impl BlockRngCore for OneNg {
type Item = u32;
type Results = [u32; 16];

fn generate(&mut self, results: &mut Self::Results) {
for elem in results.iter_mut() {
*elem = 1;
}
}
}

pub fn one_rng() -> BlockRng<OneNg> {
BlockRng::<OneNg>::new(OneNg {})
}

ed255 marked this conversation as resolved.
Show resolved Hide resolved
/// Gets the hex representation of the keccak hash of the input data
pub fn keccak_hex<D: AsRef<[u8]>>(data: D) -> String {
let mut hash = [0u8; 32];
let mut hasher = tiny_keccak::Keccak::v256();
hasher.update(data.as_ref());
hasher.finalize(&mut hash);
hex::encode(hash)
}
1 change: 0 additions & 1 deletion halo2_frontend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ bits = ["halo2curves/bits"]
gadget-traces = ["backtrace"]
thread-safe-region = []
circuit-params = []
heap-profiling = []
cost-estimator = ["serde", "serde_derive"]
derive_serde = ["halo2curves/derive_serde"]

Expand Down
3 changes: 2 additions & 1 deletion halo2_proofs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ gumdrop = "0.8"
proptest = "1"
dhat = "0.3.2"
serde_json = "1"
halo2_debug = { path = "../halo2_debug" }

[target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dev-dependencies]
getrandom = { version = "0.2", features = ["js"] }
Expand All @@ -81,9 +82,9 @@ thread-safe-region = ["halo2_frontend/thread-safe-region"]
sanity-checks = ["halo2_backend/sanity-checks"]
batch = ["rand_core/getrandom", "halo2_backend/batch"]
circuit-params = ["halo2_frontend/circuit-params"]
heap-profiling = ["halo2_frontend/heap-profiling"]
cost-estimator = ["halo2_frontend/cost-estimator"]
derive_serde = ["halo2curves/derive_serde", "halo2_frontend/derive_serde", "halo2_backend/derive_serde"]
vector-tests = []
Copy link
Member

Choose a reason for hiding this comment

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

We need to make sure this feature is enabled on test in the CI (this way we can detect the PRs that change the proof output). Is this the case?
If not, maybe we can add vector-tests as a default feature?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that this can be removed at all now, just keeping not(coverage) , let's me check again.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ed255 the main point here is that results can differ when turning some features (like circuit-params), so we want a feature that is only activated when we test it with --all-features. I keep it as is.

Copy link
Member

Choose a reason for hiding this comment

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

I see, but then we're not testing this in the CI. We're only testing these sets of features:

  • batch,dev-graph,gadget-traces
  • batch,dev-graph,gadget-traces,test-dev-graph,thread-safe-region,sanity-checks,circuit-params

I think we should have a job in the CI that runs the tests with vector-tests enabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

Another option is to change the "all features" in CI to --all-features instead "--features batch,dev-graph,gadget-traces,test-dev-graph,thread-safe-region,sanity-checks,circuit-params". Not sure, testing with --all-features is more straighforward that specifying them manually if there's no conflicting options.


[lib]
bench = false
Expand Down
2 changes: 1 addition & 1 deletion halo2_proofs/benches/plonk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ fn criterion_benchmark(c: &mut Criterion) {
}

fn keygen(k: u32) -> (ParamsIPA<EqAffine>, ProvingKey<EqAffine>) {
let params: ParamsIPA<EqAffine> = ParamsIPA::new(k);
let params: ParamsIPA<EqAffine> = ParamsIPA::new(k, OsRng);
let empty_circuit: MyCircuit<Fp> = MyCircuit {
a: Value::unknown(),
k,
Expand Down
43 changes: 21 additions & 22 deletions halo2_proofs/tests/compress_selectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use halo2_proofs::poly::Rotation;
use halo2_backend::transcript::{
Blake2bRead, Blake2bWrite, Challenge255, TranscriptReadBuffer, TranscriptWriterBuffer,
};
use halo2_debug::one_rng;
use halo2_middleware::zal::impls::{H2cEngine, PlonkEngineConfig};
use halo2_proofs::arithmetic::Field;
use halo2_proofs::plonk::{
Expand All @@ -20,22 +21,6 @@ use halo2_proofs::poly::kzg::commitment::{KZGCommitmentScheme, ParamsKZG};
use halo2_proofs::poly::kzg::multiopen::{ProverSHPLONK, VerifierSHPLONK};
use halo2_proofs::poly::kzg::strategy::SingleStrategy;
use halo2curves::bn256::{Bn256, Fr, G1Affine};
use rand_core::block::BlockRng;
use rand_core::block::BlockRngCore;

// One number generator, that can be used as a deterministic Rng, outputing fixed values.
pub struct OneNg {}

impl BlockRngCore for OneNg {
type Item = u32;
type Results = [u32; 16];

fn generate(&mut self, results: &mut Self::Results) {
for elem in results.iter_mut() {
*elem = 1;
}
}
}

#[derive(Debug, Clone)]
struct MyCircuitConfig {
Expand Down Expand Up @@ -344,7 +329,7 @@ impl<F: Field> Circuit<F> for MyCircuit<F> {
fn test_mycircuit(
vk_keygen_compress_selectors: bool,
pk_keygen_compress_selectors: bool,
) -> Result<(), halo2_proofs::plonk::Error> {
) -> Result<Vec<u8>, halo2_proofs::plonk::Error> {
let engine = PlonkEngineConfig::new()
.set_curve::<G1Affine>()
.set_msm(H2cEngine::new())
Expand All @@ -357,7 +342,7 @@ fn test_mycircuit(
};

// Setup
let mut rng = BlockRng::new(OneNg {});
let mut rng = one_rng();
let params = ParamsKZG::<Bn256>::setup(k, &mut rng);
let verifier_params = params.verifier_params();
let vk = keygen_vk_custom(&params, &circuit, vk_keygen_compress_selectors)?;
Expand Down Expand Up @@ -395,7 +380,9 @@ fn test_mycircuit(
&[instances_slice],
&mut verifier_transcript,
)
.map_err(halo2_proofs::plonk::Error::Backend)
.map_err(halo2_proofs::plonk::Error::Backend)?;

Ok(proof)
}

/*
Expand Down Expand Up @@ -436,12 +423,24 @@ How the `compress_selectors` works in `MyCircuit` under the hood:
*/

#[test]
fn test_success() {
fn test_success() -> Result<(), halo2_proofs::plonk::Error> {
// vk & pk keygen both WITH compress
assert!(test_mycircuit(true, true).is_ok());
let _proof = test_mycircuit(true, true)?;
#[cfg(all(feature = "vector-tests", not(coverage)))]
assert_eq!(
"088a7a9782efb2b9518e3c181ab126c9d621feb83b61dc21c93aaf4acf1c818c",
halo2_debug::keccak_hex(_proof),
adria0 marked this conversation as resolved.
Show resolved Hide resolved
);

// vk & pk keygen both WITHOUT compress
assert!(test_mycircuit(false, false).is_ok());
let _proof = test_mycircuit(false, false)?;
#[cfg(all(feature = "vector-tests", not(coverage)))]
assert_eq!(
"b799363093b85f956c9f68d001e99bd68901147dc4059eaf70f9cacbf02b6886",
halo2_debug::keccak_hex(_proof),
);

Ok(())
}

#[should_panic]
Expand Down
Loading
Loading