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 all commits
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
7 changes: 3 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@ jobs:
os: [ubuntu-latest, windows-latest, macOS-latest]
include:
- feature_set: basic
features: batch,dev-graph,gadget-traces
features: --features batch,dev-graph,gadget-traces
- feature_set: all
features: batch,dev-graph,gadget-traces,test-dev-graph,thread-safe-region,sanity-checks,circuit-params,lookup-any-sanity-checks

features: --all-features
steps:
- uses: actions/checkout@v3
- uses: actions-rs/toolchain@v1
Expand All @@ -30,7 +29,7 @@ jobs:
uses: actions-rs/cargo@v1
with:
command: test
args: --verbose --release --workspace --no-default-features --features "${{ matrix.features }}"
args: --verbose --release --workspace --no-default-features ${{ matrix.features }}

examples:
name: Run the examples
Expand Down
8 changes: 8 additions & 0 deletions halo2_debug/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,11 @@ ff = "0.13"
halo2curves = { version = "0.6.1", default-features = false }
num-bigint = "0.4.5"
halo2_middleware = { path = "../halo2_middleware" }
tiny-keccak = { version = "2.0.2", features=["keccak"] }
hex = "0.4.3"
rand_core = "0.6.4"
rand_chacha = "0.3"
rayon = "1.8"

[features]
vector-tests = []
37 changes: 37 additions & 0 deletions halo2_debug/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1 +1,38 @@
use rand_chacha::ChaCha20Rng;
use rand_core::SeedableRng;
use tiny_keccak::Hasher;

pub fn test_rng() -> ChaCha20Rng {
ChaCha20Rng::seed_from_u64(0xdeadbeef)
}

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)
}

/// When the feature `vector-tests` is enabled, executes the test in a single thread and checks the result against the expected value.
/// When the feature `vector-tests` is disabled, just executes the test.
pub fn test_result<F: FnOnce() -> Vec<u8> + Send>(test: F, _expected: &str) -> Vec<u8> {
#[cfg(feature = "vector-tests")]
let result = rayon::ThreadPoolBuilder::new()
.num_threads(1)
.build()
.unwrap()
.install(|| {
let result = test();
assert_eq!(_expected, keccak_hex(result.clone()),);
result
});

#[cfg(not(feature = "vector-tests"))]
let result = test();

result
}

pub mod display;
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"]
lookup-any-sanity-checks = []
Expand Down
2 changes: 1 addition & 1 deletion halo2_proofs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -82,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.

lookup-any-sanity-checks = ["halo2_frontend/lookup-any-sanity-checks"]

[lib]
Expand Down
1 change: 0 additions & 1 deletion halo2_proofs/src/plonk/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ where
}
Ok(prover.create_proof()?)
}

/// This creates a proof for the provided `circuit` when given the public
/// parameters `params` and the proving key [`ProvingKey`] that was
/// generated previously for the same circuit. The provided `instances`
Expand Down
40 changes: 18 additions & 22 deletions halo2_proofs/tests/compress_selectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::marker::PhantomData;

use ff::PrimeField;
use halo2_debug::display::expr_disp_names;
use halo2_debug::{test_result, test_rng};
use halo2_frontend::circuit::compile_circuit;
use halo2_frontend::plonk::Error;
use halo2_proofs::circuit::{Cell, Layouter, SimpleFloorPlanner, Value};
Expand All @@ -23,22 +24,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 @@ -351,7 +336,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 @@ -363,8 +348,9 @@ fn test_mycircuit(
constant: Fr::one(),
};

let mut rng = test_rng();

// Setup
let mut rng = BlockRng::new(OneNg {});
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 @@ -398,7 +384,9 @@ fn test_mycircuit(
instances.as_slice(),
&mut verifier_transcript,
)
.map_err(halo2_proofs::plonk::Error::Backend)
.map_err(halo2_proofs::plonk::Error::Backend)?;

Ok(proof)
}

/*
Expand Down Expand Up @@ -496,12 +484,20 @@ fn test_compress_gates() {
}

#[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());
test_result(
|| test_mycircuit(true, true).expect("should pass"),
"8326140d1873a91630d439a8812d1f104667144e03e0cd5c59eb358ae5d1a4eb",
);

// vk & pk keygen both WITHOUT compress
assert!(test_mycircuit(false, false).is_ok());
test_result(
|| test_mycircuit(false, false).expect("should pass"),
"73dd4c3c9c51d55dc8cf68ca2b5d8acdb40ed44bc8a88d718325bc0023688f64",
);

Ok(())
}

#[should_panic]
Expand Down
Loading
Loading