From 16b3f2ae53ec0981174d8a32c97b79b15baea30a Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 10:23:43 +0200 Subject: [PATCH 01/14] Have a backend by machine --- backend/src/composite/mod.rs | 63 ++++++++++++++++++++++++------------ backend/src/lib.rs | 5 +++ 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/backend/src/composite/mod.rs b/backend/src/composite/mod.rs index d1f28966f..7d2f5cc1b 100644 --- a/backend/src/composite/mod.rs +++ b/backend/src/composite/mod.rs @@ -1,8 +1,8 @@ -use std::{io, marker::PhantomData}; +use std::{collections::BTreeMap, io, marker::PhantomData}; use powdr_ast::analyzed::Analyzed; use powdr_executor::witgen::WitgenCallback; -use powdr_number::FieldElement; +use powdr_number::{DegreeType, FieldElement}; use crate::{Backend, BackendFactory, BackendOptions, Error, Proof}; @@ -31,21 +31,36 @@ impl> BackendFactory for CompositeBacke verification_app_key: Option<&mut dyn std::io::Read>, backend_options: BackendOptions, ) -> Result + 'a>, Error> { - let backend: Box + 'a> = self.factory.create( - pil, - fixed, - output_dir, - setup, - verification_key, - verification_app_key, - backend_options, - )?; - Ok(Box::new(CompositeBackend { backend })) + if setup.is_some() || verification_key.is_some() || verification_app_key.is_some() { + unimplemented!(); + } + + let backend_by_machine = ["main"] + .iter() + .map(|machine_name| { + let backend = self.factory.create( + pil, + fixed, + output_dir, + // TODO: Handle setup, verification_key, verification_app_key + None, + None, + None, + backend_options.clone(), + ); + backend.map(|backend| (machine_name.to_string(), backend)) + }) + .collect::, _>>()?; + Ok(Box::new(CompositeBackend { backend_by_machine })) + } + + fn generate_setup(&self, _size: DegreeType, _output: &mut dyn io::Write) -> Result<(), Error> { + Err(Error::NoSetupAvailable) } } pub(crate) struct CompositeBackend<'a, F: FieldElement> { - backend: Box + 'a>, + backend_by_machine: BTreeMap + 'a>>, } // TODO: This just forwards to the backend for now. In the future this should: @@ -60,22 +75,28 @@ impl<'a, F: FieldElement> Backend<'a, F> for CompositeBackend<'a, F> { prev_proof: Option, witgen_callback: WitgenCallback, ) -> Result { - self.backend.prove(witness, prev_proof, witgen_callback) + self.backend_by_machine + .get("main") + .unwrap() + .prove(witness, prev_proof, witgen_callback) } fn verify(&self, _proof: &[u8], instances: &[Vec]) -> Result<(), Error> { - self.backend.verify(_proof, instances) + self.backend_by_machine + .get("main") + .unwrap() + .verify(_proof, instances) } - fn export_setup(&self, output: &mut dyn io::Write) -> Result<(), Error> { - self.backend.export_setup(output) + fn export_setup(&self, _output: &mut dyn io::Write) -> Result<(), Error> { + unimplemented!() } - fn export_verification_key(&self, output: &mut dyn io::Write) -> Result<(), Error> { - self.backend.export_verification_key(output) + fn export_verification_key(&self, _output: &mut dyn io::Write) -> Result<(), Error> { + unimplemented!(); } - fn export_ethereum_verifier(&self, output: &mut dyn io::Write) -> Result<(), Error> { - self.backend.export_ethereum_verifier(output) + fn export_ethereum_verifier(&self, _output: &mut dyn io::Write) -> Result<(), Error> { + unimplemented!(); } } diff --git a/backend/src/lib.rs b/backend/src/lib.rs index d75187189..2e9ad7cdb 100644 --- a/backend/src/lib.rs +++ b/backend/src/lib.rs @@ -28,6 +28,8 @@ pub enum BackendType { EStarkPolygon, #[strum(serialize = "estark-starky")] EStarkStarky, + #[strum(serialize = "estark-composite")] + EStarkComposite, #[strum(serialize = "estark-dump")] EStarkDump, } @@ -51,6 +53,9 @@ impl BackendType { #[cfg(feature = "estark-polygon")] BackendType::EStarkPolygon => Box::new(estark::polygon_wrapper::Factory), BackendType::EStarkStarky => Box::new(estark::starky_wrapper::Factory), + BackendType::EStarkComposite => Box::new(composite::CompositeBackendFactory::new( + estark::starky_wrapper::Factory, + )), BackendType::EStarkDump => Box::new(estark::DumpFactory), } } From 89a7e1b0a22966552b71340c6882ba57c6d875a3 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 10:37:41 +0200 Subject: [PATCH 02/14] Set output dir per machine --- backend/src/composite/mod.rs | 8 ++++++-- backend/src/estark/mod.rs | 21 +++++++++++---------- backend/src/estark/starky_wrapper.rs | 3 ++- backend/src/halo2/mod.rs | 7 ++++--- backend/src/lib.rs | 4 ++-- pipeline/src/pipeline.rs | 12 ++++++------ pipeline/src/test_util.rs | 2 +- 7 files changed, 32 insertions(+), 25 deletions(-) diff --git a/backend/src/composite/mod.rs b/backend/src/composite/mod.rs index 7d2f5cc1b..56badf5d1 100644 --- a/backend/src/composite/mod.rs +++ b/backend/src/composite/mod.rs @@ -1,4 +1,4 @@ -use std::{collections::BTreeMap, io, marker::PhantomData}; +use std::{collections::BTreeMap, io, marker::PhantomData, path::PathBuf}; use powdr_ast::analyzed::Analyzed; use powdr_executor::witgen::WitgenCallback; @@ -25,7 +25,7 @@ impl> BackendFactory for CompositeBacke &self, pil: &'a Analyzed, fixed: &'a [(String, Vec)], - output_dir: Option<&'a std::path::Path>, + output_dir: Option, setup: Option<&mut dyn std::io::Read>, verification_key: Option<&mut dyn std::io::Read>, verification_app_key: Option<&mut dyn std::io::Read>, @@ -38,6 +38,10 @@ impl> BackendFactory for CompositeBacke let backend_by_machine = ["main"] .iter() .map(|machine_name| { + let output_dir = output_dir + .clone() + .map(|output_dir| output_dir.join(machine_name)); + println!("Output dir: {:?}", output_dir); let backend = self.factory.create( pil, fixed, diff --git a/backend/src/estark/mod.rs b/backend/src/estark/mod.rs index 29168a708..420373b67 100644 --- a/backend/src/estark/mod.rs +++ b/backend/src/estark/mod.rs @@ -116,13 +116,13 @@ fn first_step_fixup<'a, F: FieldElement>( (pil, patched_constants) } -struct EStarkFilesCommon<'a, F: FieldElement> { +struct EStarkFilesCommon { degree: DegreeType, pil: PIL, /// If this field is present, it means the constants were patched with /// "main.first_step" column and must be written again to a file. patched_constants: Option)>>, - output_dir: Option<&'a Path>, + output_dir: Option, proof_type: ProofType, } @@ -135,11 +135,11 @@ fn write_json_file(path: &Path, data: &T) -> Result<(), E Ok(()) } -impl<'a, F: FieldElement> EStarkFilesCommon<'a, F> { +impl EStarkFilesCommon { fn create( - analyzed: &'a Analyzed, - fixed: &'a [(String, Vec)], - output_dir: Option<&'a Path>, + analyzed: &Analyzed, + fixed: &[(String, Vec)], + output_dir: Option, setup: Option<&mut dyn std::io::Read>, verification_key: Option<&mut dyn std::io::Read>, verification_app_key: Option<&mut dyn std::io::Read>, @@ -176,7 +176,7 @@ struct ProverInputFilePaths { contraints: PathBuf, } -impl<'a, F: FieldElement> EStarkFilesCommon<'a, F> { +impl EStarkFilesCommon { /// Write the files in the EStark Polygon format. fn write_files(&self, output_dir: &Path) -> Result { let paths = ProverInputFilePaths { @@ -216,7 +216,7 @@ impl BackendFactory for DumpFactory { &self, analyzed: &'a Analyzed, fixed: &'a [(String, Vec)], - output_dir: Option<&'a Path>, + output_dir: Option, setup: Option<&mut dyn std::io::Read>, verification_key: Option<&mut dyn std::io::Read>, verification_app_key: Option<&mut dyn std::io::Read>, @@ -235,9 +235,9 @@ impl BackendFactory for DumpFactory { } /// A backend that just dumps the files to the output directory. -struct DumpBackend<'a, F: FieldElement>(EStarkFilesCommon<'a, F>); +struct DumpBackend(EStarkFilesCommon); -impl<'a, F: FieldElement> Backend<'a, F> for DumpBackend<'a, F> { +impl<'a, F: FieldElement> Backend<'a, F> for DumpBackend { fn prove( &self, _witness: &[(String, Vec)], @@ -252,6 +252,7 @@ impl<'a, F: FieldElement> Backend<'a, F> for DumpBackend<'a, F> { let output_dir = self .0 .output_dir + .as_ref() .ok_or(Error::BackendError("output_dir is None".to_owned()))?; self.0.write_files(output_dir)?; diff --git a/backend/src/estark/starky_wrapper.rs b/backend/src/estark/starky_wrapper.rs index 5527c8c6c..4ca379229 100644 --- a/backend/src/estark/starky_wrapper.rs +++ b/backend/src/estark/starky_wrapper.rs @@ -1,3 +1,4 @@ +use std::path::PathBuf; use std::time::Instant; use std::{borrow::Cow, io}; @@ -26,7 +27,7 @@ impl BackendFactory for Factory { &self, pil: &'a Analyzed, fixed: &'a [(String, Vec)], - _output_dir: Option<&std::path::Path>, + _output_dir: Option, setup: Option<&mut dyn std::io::Read>, verification_key: Option<&mut dyn std::io::Read>, verification_app_key: Option<&mut dyn std::io::Read>, diff --git a/backend/src/halo2/mod.rs b/backend/src/halo2/mod.rs index ac7dcc955..bc30c2e6c 100644 --- a/backend/src/halo2/mod.rs +++ b/backend/src/halo2/mod.rs @@ -1,6 +1,7 @@ #![deny(clippy::print_stdout)] -use std::{io, path::Path}; +use std::io; +use std::path::PathBuf; use crate::{Backend, BackendFactory, BackendOptions, Error, Proof}; use powdr_ast::analyzed::Analyzed; @@ -75,7 +76,7 @@ impl BackendFactory for Halo2ProverFactory { &self, pil: &'a Analyzed, fixed: &'a [(String, Vec)], - _output_dir: Option<&'a Path>, + _output_dir: Option, setup: Option<&mut dyn io::Read>, verification_key: Option<&mut dyn io::Read>, verification_app_key: Option<&mut dyn io::Read>, @@ -181,7 +182,7 @@ impl BackendFactory for Halo2MockFactory { &self, pil: &'a Analyzed, fixed: &'a [(String, Vec)], - _output_dir: Option<&'a Path>, + _output_dir: Option, setup: Option<&mut dyn io::Read>, verification_key: Option<&mut dyn io::Read>, verification_app_key: Option<&mut dyn io::Read>, diff --git a/backend/src/lib.rs b/backend/src/lib.rs index 2e9ad7cdb..e869ebd14 100644 --- a/backend/src/lib.rs +++ b/backend/src/lib.rs @@ -9,7 +9,7 @@ mod composite; use powdr_ast::analyzed::Analyzed; use powdr_executor::witgen::WitgenCallback; use powdr_number::{DegreeType, FieldElement}; -use std::{io, path::Path}; +use std::{io, path::PathBuf}; use strum::{Display, EnumString, EnumVariantNames}; #[derive(Clone, EnumString, EnumVariantNames, Display, Copy)] @@ -100,7 +100,7 @@ pub trait BackendFactory { &self, pil: &'a Analyzed, fixed: &'a [(String, Vec)], - output_dir: Option<&'a Path>, + output_dir: Option, setup: Option<&mut dyn io::Read>, verification_key: Option<&mut dyn io::Read>, verification_app_key: Option<&mut dyn io::Read>, diff --git a/pipeline/src/pipeline.rs b/pipeline/src/pipeline.rs index 14cc4e96a..cb75dfcfa 100644 --- a/pipeline/src/pipeline.rs +++ b/pipeline/src/pipeline.rs @@ -914,7 +914,7 @@ impl Pipeline { .create( pil.borrow(), &fixed_cols[..], - self.output_dir(), + self.output_dir.clone(), setup.as_io_read(), vkey.as_io_read(), vkey_app.as_io_read(), @@ -950,8 +950,8 @@ impl Pipeline { Ok(self.artifact.proof.as_ref().unwrap()) } - pub fn output_dir(&self) -> Option<&Path> { - self.output_dir.as_ref().map(|p| p.as_ref()) + pub fn output_dir(&self) -> &Option { + &self.output_dir } pub fn is_force_overwrite(&self) -> bool { @@ -996,7 +996,7 @@ impl Pipeline { .create( pil.borrow(), &fixed_cols[..], - self.output_dir(), + self.output_dir.clone(), setup_file .as_mut() .map(|file| file as &mut dyn std::io::Read), @@ -1050,7 +1050,7 @@ impl Pipeline { .create( pil.borrow(), &fixed_cols[..], - self.output_dir(), + self.output_dir.clone(), setup_file .as_mut() .map(|file| file as &mut dyn std::io::Read), @@ -1096,7 +1096,7 @@ impl Pipeline { .create( pil.borrow(), &fixed_cols[..], - self.output_dir(), + self.output_dir.clone(), setup_file .as_mut() .map(|file| file as &mut dyn std::io::Read), diff --git a/pipeline/src/test_util.rs b/pipeline/src/test_util.rs index 1d39808b6..f8ace8103 100644 --- a/pipeline/src/test_util.rs +++ b/pipeline/src/test_util.rs @@ -60,7 +60,7 @@ pub fn verify_pipeline( pipeline.compute_proof().unwrap(); - verify(pipeline.output_dir().unwrap()) + verify(pipeline.output_dir().as_ref().unwrap()) } pub fn gen_estark_proof(file_name: &str, inputs: Vec) { From e6858005dfdd5e207ca1247dd36ccbd4c1b172f3 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 10:45:39 +0200 Subject: [PATCH 03/14] Add all backends --- backend/src/composite/mod.rs | 4 +++- backend/src/lib.rs | 27 +++++++++++++++++++++++---- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/backend/src/composite/mod.rs b/backend/src/composite/mod.rs index 56badf5d1..aca2b98a8 100644 --- a/backend/src/composite/mod.rs +++ b/backend/src/composite/mod.rs @@ -41,7 +41,9 @@ impl> BackendFactory for CompositeBacke let output_dir = output_dir .clone() .map(|output_dir| output_dir.join(machine_name)); - println!("Output dir: {:?}", output_dir); + if let Some(ref output_dir) = output_dir { + std::fs::create_dir_all(output_dir)?; + } let backend = self.factory.create( pil, fixed, diff --git a/backend/src/lib.rs b/backend/src/lib.rs index e869ebd14..8290eae9d 100644 --- a/backend/src/lib.rs +++ b/backend/src/lib.rs @@ -18,6 +18,9 @@ pub enum BackendType { #[strum(serialize = "halo2")] Halo2, #[cfg(feature = "halo2")] + #[strum(serialize = "halo2-composite")] + Halo2Composite, + #[cfg(feature = "halo2")] #[strum(serialize = "halo2-mock")] Halo2Mock, #[cfg(feature = "halo2")] @@ -26,12 +29,17 @@ pub enum BackendType { #[cfg(feature = "estark-polygon")] #[strum(serialize = "estark-polygon")] EStarkPolygon, + #[cfg(feature = "estark-polygon")] + #[strum(serialize = "estark-polygon-composite")] + EStarkPolygonComposite, #[strum(serialize = "estark-starky")] EStarkStarky, #[strum(serialize = "estark-composite")] - EStarkComposite, + EStarkStarkyComposite, #[strum(serialize = "estark-dump")] EStarkDump, + #[strum(serialize = "estark-dump-composite")] + EStarkDumpComposite, } pub type BackendOptions = String; @@ -45,6 +53,10 @@ impl BackendType { #[cfg(feature = "halo2")] BackendType::Halo2 => Box::new(halo2::Halo2ProverFactory), #[cfg(feature = "halo2")] + BackendType::Halo2Composite => Box::new(composite::CompositeBackendFactory::new( + halo2::Halo2ProverFactory, + )), + #[cfg(feature = "halo2")] BackendType::Halo2Mock => Box::new(halo2::Halo2MockFactory), #[cfg(feature = "halo2")] BackendType::Halo2MockComposite => Box::new(composite::CompositeBackendFactory::new( @@ -52,11 +64,18 @@ impl BackendType { )), #[cfg(feature = "estark-polygon")] BackendType::EStarkPolygon => Box::new(estark::polygon_wrapper::Factory), + #[cfg(feature = "estark-polygon")] + BackendType::EStarkPolygonComposite => Box::new( + composite::CompositeBackendFactory::new(estark::polygon_wrapper::Factory), + ), BackendType::EStarkStarky => Box::new(estark::starky_wrapper::Factory), - BackendType::EStarkComposite => Box::new(composite::CompositeBackendFactory::new( - estark::starky_wrapper::Factory, - )), + BackendType::EStarkStarkyComposite => Box::new( + composite::CompositeBackendFactory::new(estark::starky_wrapper::Factory), + ), BackendType::EStarkDump => Box::new(estark::DumpFactory), + BackendType::EStarkDumpComposite => { + Box::new(composite::CompositeBackendFactory::new(estark::DumpFactory)) + } } } } From 20b559b8499025efae13beecb94a1e31be414dac Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 11:04:42 +0200 Subject: [PATCH 04/14] Test Composite wrapper --- pipeline/src/test_util.rs | 46 ++++++++++++++++++++++++++++--------- pipeline/tests/powdr_std.rs | 4 +++- riscv/tests/common/mod.rs | 4 +--- riscv/tests/instructions.rs | 9 +------- riscv/tests/riscv.rs | 21 +++-------------- 5 files changed, 43 insertions(+), 41 deletions(-) diff --git a/pipeline/src/test_util.rs b/pipeline/src/test_util.rs index f8ace8103..1d8b10463 100644 --- a/pipeline/src/test_util.rs +++ b/pipeline/src/test_util.rs @@ -25,7 +25,7 @@ pub fn verify_test_file( .from_file(resolve_test_file(file_name)) .with_prover_inputs(inputs) .add_external_witness_values(external_witness_values); - verify_pipeline(pipeline, BackendType::EStarkDump) + verify_pipeline(pipeline) } pub fn verify_asm_string( @@ -44,14 +44,12 @@ pub fn verify_asm_string( pipeline = pipeline.add_data_vec(&data); } - verify_pipeline(pipeline, BackendType::EStarkDump).unwrap(); + verify_pipeline(pipeline).unwrap(); } -pub fn verify_pipeline( - pipeline: Pipeline, - backend: BackendType, -) -> Result<(), String> { - let mut pipeline = pipeline.with_backend(backend, None); +pub fn verify_pipeline(pipeline: Pipeline) -> Result<(), String> { + // TODO: Also test EStarkDumpComposite + let mut pipeline = pipeline.with_backend(BackendType::EStarkDump, None); let tmp_dir = mktemp::Temp::new_dir().unwrap(); if pipeline.output_dir().is_none() { @@ -73,6 +71,13 @@ pub fn gen_estark_proof(file_name: &str, inputs: Vec) { pipeline.clone().compute_proof().unwrap(); + // Also test composite backend: + pipeline + .clone() + .with_backend(powdr_backend::BackendType::EStarkStarkyComposite, None) + .compute_proof() + .unwrap(); + // Repeat the proof generation, but with an externally generated verification key // Verification Key @@ -110,13 +115,22 @@ pub fn test_halo2(file_name: &str, inputs: Vec) { .compute_proof() .unwrap(); + // Also generate a proof with the composite backend + Pipeline::default() + .from_file(resolve_test_file(file_name)) + .with_prover_inputs(inputs.clone()) + .with_backend(powdr_backend::BackendType::Halo2MockComposite, None) + .compute_proof() + .unwrap(); + // `gen_halo2_proof` is rather slow, because it computes two Halo2 proofs. // Therefore, we only run it in the nightly tests. let is_nightly_test = env::var("IS_NIGHTLY_TEST") .map(|v| v == "true") .unwrap_or(false); if is_nightly_test { - gen_halo2_proof(file_name, inputs) + gen_halo2_proof(file_name, inputs.clone(), false); + gen_halo2_proof(file_name, inputs, true); } } @@ -124,17 +138,27 @@ pub fn test_halo2(file_name: &str, inputs: Vec) { pub fn test_halo2(_file_name: &str, _inputs: Vec) {} #[cfg(feature = "halo2")] -pub fn gen_halo2_proof(file_name: &str, inputs: Vec) { +pub fn gen_halo2_proof(file_name: &str, inputs: Vec, composite: bool) { let tmp_dir = mktemp::Temp::new_dir().unwrap(); + let backend = if composite { + powdr_backend::BackendType::Halo2Composite + } else { + powdr_backend::BackendType::Halo2 + }; let mut pipeline = Pipeline::default() .with_tmp_output(&tmp_dir) .from_file(resolve_test_file(file_name)) .with_prover_inputs(inputs) - .with_backend(powdr_backend::BackendType::Halo2, None); + .with_backend(backend, None); // Generate a proof with the setup and verification key generated on the fly pipeline.clone().compute_proof().unwrap(); + if composite { + // Providing a previously computed setup file is not supported yet + return; + } + // Repeat the proof generation, but with an externally generated setup and verification key let pil = pipeline.compute_optimized_pil().unwrap(); @@ -229,7 +253,7 @@ pub fn assert_proofs_fail_for_invalid_witnesses_pilcom( .from_file(resolve_test_file(file_name)) .set_witness(convert_witness(witness)); - assert!(verify_pipeline(pipeline.clone(), BackendType::EStarkDump).is_err()); + assert!(verify_pipeline(pipeline.clone()).is_err()); } pub fn assert_proofs_fail_for_invalid_witnesses_estark( diff --git a/pipeline/tests/powdr_std.rs b/pipeline/tests/powdr_std.rs index 0c165d1a9..687f9debd 100644 --- a/pipeline/tests/powdr_std.rs +++ b/pipeline/tests/powdr_std.rs @@ -20,7 +20,9 @@ fn poseidon_bn254_test() { // `test_halo2` only does a mock proof in the PR tests. // This makes sure we test the whole proof generation for one example // file even in the PR tests. - gen_halo2_proof(f, Default::default()); + gen_halo2_proof(f, Default::default(), false); + // Also test the composite backend + gen_halo2_proof(f, Default::default(), true); } #[test] diff --git a/riscv/tests/common/mod.rs b/riscv/tests/common/mod.rs index 00f946fc8..7a1061832 100644 --- a/riscv/tests/common/mod.rs +++ b/riscv/tests/common/mod.rs @@ -1,4 +1,3 @@ -use powdr_backend::BackendType; use powdr_number::GoldilocksField; use powdr_pipeline::{test_util::verify_pipeline, Pipeline}; use std::path::PathBuf; @@ -9,7 +8,6 @@ pub fn verify_riscv_asm_string( contents: &str, inputs: Vec, data: Option>, - backend: BackendType, ) { let temp_dir = mktemp::Temp::new_dir().unwrap().release(); @@ -33,5 +31,5 @@ pub fn verify_riscv_asm_string( powdr_riscv_executor::ExecMode::Fast, Default::default(), ); - verify_pipeline(pipeline, backend).unwrap(); + verify_pipeline(pipeline).unwrap(); } diff --git a/riscv/tests/instructions.rs b/riscv/tests/instructions.rs index 32a5c39c9..99e618bdd 100644 --- a/riscv/tests/instructions.rs +++ b/riscv/tests/instructions.rs @@ -2,7 +2,6 @@ mod common; mod instruction_tests { use crate::common::verify_riscv_asm_string; - use powdr_backend::BackendType; use powdr_number::GoldilocksField; use powdr_riscv::asm::compile; use powdr_riscv::Runtime; @@ -16,13 +15,7 @@ mod instruction_tests { false, ); - verify_riscv_asm_string::<()>( - &format!("{name}.asm"), - &powdr_asm, - Default::default(), - None, - BackendType::EStarkDump, - ); + verify_riscv_asm_string::<()>(&format!("{name}.asm"), &powdr_asm, Default::default(), None); } include!(concat!(env!("OUT_DIR"), "/instruction_tests.rs")); diff --git a/riscv/tests/riscv.rs b/riscv/tests/riscv.rs index 8e53f052e..99523ec75 100644 --- a/riscv/tests/riscv.rs +++ b/riscv/tests/riscv.rs @@ -36,7 +36,7 @@ pub fn test_continuations(case: &str) { // computing the constants file. let mut pipeline = pipeline.with_backend(BackendType::EStarkDump, None); pipeline.compute_proof().unwrap(); - verify(pipeline.output_dir().unwrap()).unwrap(); + verify(pipeline.output_dir().as_ref().unwrap()).unwrap(); Ok(()) }; @@ -337,17 +337,8 @@ fn many_chunks_memory() { } fn verify_riscv_crate(case: &str, inputs: Vec, runtime: &Runtime) { - verify_riscv_crate_with_backend(case, inputs, runtime, BackendType::EStarkDump) -} - -fn verify_riscv_crate_with_backend( - case: &str, - inputs: Vec, - runtime: &Runtime, - backend: BackendType, -) { let powdr_asm = compile_riscv_crate::(case, runtime); - verify_riscv_asm_string::<()>(&format!("{case}.asm"), &powdr_asm, inputs, None, backend); + verify_riscv_asm_string::<()>(&format!("{case}.asm"), &powdr_asm, inputs, None); } fn verify_riscv_crate_with_data( @@ -358,13 +349,7 @@ fn verify_riscv_crate_with_data( ) { let powdr_asm = compile_riscv_crate::(case, runtime); - verify_riscv_asm_string( - &format!("{case}.asm"), - &powdr_asm, - inputs, - Some(data), - BackendType::EStarkDump, - ); + verify_riscv_asm_string(&format!("{case}.asm"), &powdr_asm, inputs, Some(data)); } fn compile_riscv_crate(case: &str, runtime: &Runtime) -> String { From a6cfef0811ab8d588c21299705f9c1f8fc1c2f8a Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 11:18:52 +0200 Subject: [PATCH 05/14] Bug fixed --- backend/src/estark/polygon_wrapper.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/src/estark/polygon_wrapper.rs b/backend/src/estark/polygon_wrapper.rs index 2068dfe16..149d958e7 100644 --- a/backend/src/estark/polygon_wrapper.rs +++ b/backend/src/estark/polygon_wrapper.rs @@ -15,7 +15,7 @@ impl BackendFactory for Factory { &self, analyzed: &'a Analyzed, fixed: &'a [(String, Vec)], - output_dir: Option<&'a Path>, + output_dir: Option, setup: Option<&mut dyn std::io::Read>, verification_key: Option<&mut dyn std::io::Read>, verification_app_key: Option<&mut dyn std::io::Read>, @@ -33,11 +33,11 @@ impl BackendFactory for Factory { } } -struct PolygonBackend<'a, F: FieldElement>(EStarkFilesCommon<'a, F>); +struct PolygonBackend(EStarkFilesCommon); // TODO: make both eStark backends interchangeable, from user perspective. // TODO: implement the other Backend trait methods. -impl<'a, F: FieldElement> Backend<'a, F> for PolygonBackend<'a, F> { +impl<'a, F: FieldElement> Backend<'a, F> for PolygonBackend { fn prove( &self, // Witness is taken from file written by the pipeline. From 8c7bea4b0ac385699f79900befd4d7383b2a4acf Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 11:24:21 +0200 Subject: [PATCH 06/14] Fix bug --- backend/src/estark/polygon_wrapper.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/estark/polygon_wrapper.rs b/backend/src/estark/polygon_wrapper.rs index 149d958e7..710b3d5e0 100644 --- a/backend/src/estark/polygon_wrapper.rs +++ b/backend/src/estark/polygon_wrapper.rs @@ -1,4 +1,4 @@ -use std::{fs, path::Path}; +use std::{fs, path::PathBuf}; use powdr_ast::analyzed::Analyzed; use powdr_executor::witgen::WitgenCallback; @@ -55,7 +55,7 @@ impl<'a, F: FieldElement> Backend<'a, F> for PolygonBackend { output_dir } else { tmp_dir = mktemp::Temp::new_dir()?; - tmp_dir.as_path() + tmp_dir.to_path_buf() }; let input_paths = self.0.write_files(output_dir)?; From ba45e76e06fe353fc40c62445852f04f7ac47d41 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 11:33:17 +0200 Subject: [PATCH 07/14] Bug fix --- backend/src/estark/polygon_wrapper.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/estark/polygon_wrapper.rs b/backend/src/estark/polygon_wrapper.rs index 710b3d5e0..7f2d64071 100644 --- a/backend/src/estark/polygon_wrapper.rs +++ b/backend/src/estark/polygon_wrapper.rs @@ -58,7 +58,7 @@ impl<'a, F: FieldElement> Backend<'a, F> for PolygonBackend { tmp_dir.to_path_buf() }; - let input_paths = self.0.write_files(output_dir)?; + let input_paths = self.0.write_files(&output_dir)?; let commits_path = output_dir.join("commits.bin"); @@ -68,7 +68,7 @@ impl<'a, F: FieldElement> Backend<'a, F> for PolygonBackend { &input_paths.stark_struct, &input_paths.constants, &commits_path, - output_dir, + &output_dir, ) .map_err(|e| Error::BackendError(e.to_string()))?; From 8a230f859cad09cce5c3dc2a29846a56e9264c48 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 11:39:13 +0200 Subject: [PATCH 08/14] Bug fix? --- backend/src/estark/polygon_wrapper.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/estark/polygon_wrapper.rs b/backend/src/estark/polygon_wrapper.rs index 7f2d64071..14b2ddd5f 100644 --- a/backend/src/estark/polygon_wrapper.rs +++ b/backend/src/estark/polygon_wrapper.rs @@ -51,7 +51,7 @@ impl<'a, F: FieldElement> Backend<'a, F> for PolygonBackend { } let tmp_dir; - let output_dir = if let Some(output_dir) = self.0.output_dir { + let output_dir = if let Some(output_dir) = self.0.output_dir.clone() { output_dir } else { tmp_dir = mktemp::Temp::new_dir()?; From 1962732a75fa85050ccef84f60e1fb6ff41a1853 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 11:49:31 +0200 Subject: [PATCH 09/14] Undo changes --- pipeline/src/test_util.rs | 15 +++++++++------ riscv/tests/common/mod.rs | 4 +++- riscv/tests/instructions.rs | 9 ++++++++- riscv/tests/riscv.rs | 19 +++++++++++++++++-- 4 files changed, 37 insertions(+), 10 deletions(-) diff --git a/pipeline/src/test_util.rs b/pipeline/src/test_util.rs index 1d8b10463..d62e6a1a9 100644 --- a/pipeline/src/test_util.rs +++ b/pipeline/src/test_util.rs @@ -25,7 +25,7 @@ pub fn verify_test_file( .from_file(resolve_test_file(file_name)) .with_prover_inputs(inputs) .add_external_witness_values(external_witness_values); - verify_pipeline(pipeline) + verify_pipeline(pipeline, BackendType::EStarkDump) } pub fn verify_asm_string( @@ -44,12 +44,15 @@ pub fn verify_asm_string( pipeline = pipeline.add_data_vec(&data); } - verify_pipeline(pipeline).unwrap(); + verify_pipeline(pipeline, BackendType::EStarkDump).unwrap(); } -pub fn verify_pipeline(pipeline: Pipeline) -> Result<(), String> { - // TODO: Also test EStarkDumpComposite - let mut pipeline = pipeline.with_backend(BackendType::EStarkDump, None); +pub fn verify_pipeline( + pipeline: Pipeline, + backend: BackendType, +) -> Result<(), String> { + // TODO: Also test Composite variants + let mut pipeline = pipeline.with_backend(backend, None); let tmp_dir = mktemp::Temp::new_dir().unwrap(); if pipeline.output_dir().is_none() { @@ -253,7 +256,7 @@ pub fn assert_proofs_fail_for_invalid_witnesses_pilcom( .from_file(resolve_test_file(file_name)) .set_witness(convert_witness(witness)); - assert!(verify_pipeline(pipeline.clone()).is_err()); + assert!(verify_pipeline(pipeline.clone(), BackendType::EStarkDump).is_err()); } pub fn assert_proofs_fail_for_invalid_witnesses_estark( diff --git a/riscv/tests/common/mod.rs b/riscv/tests/common/mod.rs index 7a1061832..00f946fc8 100644 --- a/riscv/tests/common/mod.rs +++ b/riscv/tests/common/mod.rs @@ -1,3 +1,4 @@ +use powdr_backend::BackendType; use powdr_number::GoldilocksField; use powdr_pipeline::{test_util::verify_pipeline, Pipeline}; use std::path::PathBuf; @@ -8,6 +9,7 @@ pub fn verify_riscv_asm_string( contents: &str, inputs: Vec, data: Option>, + backend: BackendType, ) { let temp_dir = mktemp::Temp::new_dir().unwrap().release(); @@ -31,5 +33,5 @@ pub fn verify_riscv_asm_string( powdr_riscv_executor::ExecMode::Fast, Default::default(), ); - verify_pipeline(pipeline).unwrap(); + verify_pipeline(pipeline, backend).unwrap(); } diff --git a/riscv/tests/instructions.rs b/riscv/tests/instructions.rs index 99e618bdd..32a5c39c9 100644 --- a/riscv/tests/instructions.rs +++ b/riscv/tests/instructions.rs @@ -2,6 +2,7 @@ mod common; mod instruction_tests { use crate::common::verify_riscv_asm_string; + use powdr_backend::BackendType; use powdr_number::GoldilocksField; use powdr_riscv::asm::compile; use powdr_riscv::Runtime; @@ -15,7 +16,13 @@ mod instruction_tests { false, ); - verify_riscv_asm_string::<()>(&format!("{name}.asm"), &powdr_asm, Default::default(), None); + verify_riscv_asm_string::<()>( + &format!("{name}.asm"), + &powdr_asm, + Default::default(), + None, + BackendType::EStarkDump, + ); } include!(concat!(env!("OUT_DIR"), "/instruction_tests.rs")); diff --git a/riscv/tests/riscv.rs b/riscv/tests/riscv.rs index 99523ec75..651b41457 100644 --- a/riscv/tests/riscv.rs +++ b/riscv/tests/riscv.rs @@ -337,8 +337,17 @@ fn many_chunks_memory() { } fn verify_riscv_crate(case: &str, inputs: Vec, runtime: &Runtime) { + verify_riscv_crate_with_backend(case, inputs, runtime, BackendType::EStarkDump) +} + +fn verify_riscv_crate_with_backend( + case: &str, + inputs: Vec, + runtime: &Runtime, + backend: BackendType, +) { let powdr_asm = compile_riscv_crate::(case, runtime); - verify_riscv_asm_string::<()>(&format!("{case}.asm"), &powdr_asm, inputs, None); + verify_riscv_asm_string::<()>(&format!("{case}.asm"), &powdr_asm, inputs, None, backend); } fn verify_riscv_crate_with_data( @@ -349,7 +358,13 @@ fn verify_riscv_crate_with_data( ) { let powdr_asm = compile_riscv_crate::(case, runtime); - verify_riscv_asm_string(&format!("{case}.asm"), &powdr_asm, inputs, Some(data)); + verify_riscv_asm_string( + &format!("{case}.asm"), + &powdr_asm, + inputs, + Some(data), + BackendType::EStarkDump, + ); } fn compile_riscv_crate(case: &str, runtime: &Runtime) -> String { From 8f472bdbb4aa0160b4e4463f1a7ebe09a9e46a6c Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 12:44:42 +0200 Subject: [PATCH 10/14] Speed up arith_test --- pipeline/tests/powdr_std.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pipeline/tests/powdr_std.rs b/pipeline/tests/powdr_std.rs index 687f9debd..e7534fedc 100644 --- a/pipeline/tests/powdr_std.rs +++ b/pipeline/tests/powdr_std.rs @@ -50,7 +50,16 @@ fn split_gl_test() { fn arith_test() { let f = "std/arith_test.asm"; verify_test_file(f, Default::default(), vec![]).unwrap(); - gen_estark_proof(f, Default::default()); + + // Running gen_estark_proof(f, Default::default()) + // is too slow for the PR tests. This will only create a single + // eStark proof instead of 3. + Pipeline::::default() + .from_file(resolve_test_file(f)) + .with_backend(powdr_backend::BackendType::EStarkStarky, None) + .compute_proof() + .unwrap(); + test_halo2(f, Default::default()); } From de80b2f4b0246b2c1ab5006414cf810689b3914d Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 21:09:24 +0200 Subject: [PATCH 11/14] Split function --- pipeline/src/test_util.rs | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/pipeline/src/test_util.rs b/pipeline/src/test_util.rs index d62e6a1a9..ee4402006 100644 --- a/pipeline/src/test_util.rs +++ b/pipeline/src/test_util.rs @@ -132,8 +132,8 @@ pub fn test_halo2(file_name: &str, inputs: Vec) { .map(|v| v == "true") .unwrap_or(false); if is_nightly_test { - gen_halo2_proof(file_name, inputs.clone(), false); - gen_halo2_proof(file_name, inputs, true); + gen_halo2_proof(file_name, inputs.clone()); + gen_halo2_composite_proof(file_name, inputs); } } @@ -141,27 +141,17 @@ pub fn test_halo2(file_name: &str, inputs: Vec) { pub fn test_halo2(_file_name: &str, _inputs: Vec) {} #[cfg(feature = "halo2")] -pub fn gen_halo2_proof(file_name: &str, inputs: Vec, composite: bool) { +pub fn gen_halo2_proof(file_name: &str, inputs: Vec) { let tmp_dir = mktemp::Temp::new_dir().unwrap(); - let backend = if composite { - powdr_backend::BackendType::Halo2Composite - } else { - powdr_backend::BackendType::Halo2 - }; let mut pipeline = Pipeline::default() .with_tmp_output(&tmp_dir) .from_file(resolve_test_file(file_name)) .with_prover_inputs(inputs) - .with_backend(backend, None); + .with_backend(powdr_backend::BackendType::Halo2, None); // Generate a proof with the setup and verification key generated on the fly pipeline.clone().compute_proof().unwrap(); - if composite { - // Providing a previously computed setup file is not supported yet - return; - } - // Repeat the proof generation, but with an externally generated setup and verification key let pil = pipeline.compute_optimized_pil().unwrap(); @@ -202,6 +192,21 @@ pub fn gen_halo2_proof(file_name: &str, inputs: Vec, composite: bool #[cfg(not(feature = "halo2"))] pub fn gen_halo2_proof(_file_name: &str, _inputs: Vec) {} +#[cfg(feature = "halo2")] +pub fn gen_halo2_composite_proof(file_name: &str, inputs: Vec) { + let tmp_dir = mktemp::Temp::new_dir().unwrap(); + Pipeline::default() + .with_tmp_output(&tmp_dir) + .from_file(resolve_test_file(file_name)) + .with_prover_inputs(inputs) + .with_backend(powdr_backend::BackendType::Halo2Composite, None) + .compute_proof() + .unwrap(); +} + +#[cfg(not(feature = "halo2"))] +pub fn gen_halo2_composite_proof(_file_name: &str, _inputs: Vec) {} + /// Returns the analyzed PIL containing only the std library. pub fn std_analyzed() -> Analyzed { let mut pipeline = Pipeline::default().from_asm_string(String::new(), None); From de1e2b3cdad65fcb4ec8433624648058c09c480e Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Wed, 26 Jun 2024 21:10:19 +0200 Subject: [PATCH 12/14] Bug fix --- pipeline/tests/powdr_std.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pipeline/tests/powdr_std.rs b/pipeline/tests/powdr_std.rs index e7534fedc..81e641f89 100644 --- a/pipeline/tests/powdr_std.rs +++ b/pipeline/tests/powdr_std.rs @@ -5,8 +5,7 @@ use powdr_number::{BigInt, Bn254Field, GoldilocksField}; use powdr_pil_analyzer::evaluator::Value; use powdr_pipeline::{ test_util::{ - evaluate_function, evaluate_integer_function, gen_estark_proof, gen_halo2_proof, - resolve_test_file, std_analyzed, test_halo2, verify_test_file, + evaluate_function, evaluate_integer_function, gen_estark_proof, gen_halo2_composite_proof, gen_halo2_proof, resolve_test_file, std_analyzed, test_halo2, verify_test_file }, Pipeline, }; @@ -20,9 +19,8 @@ fn poseidon_bn254_test() { // `test_halo2` only does a mock proof in the PR tests. // This makes sure we test the whole proof generation for one example // file even in the PR tests. - gen_halo2_proof(f, Default::default(), false); - // Also test the composite backend - gen_halo2_proof(f, Default::default(), true); + gen_halo2_proof(f, Default::default()); + gen_halo2_composite_proof(f, Default::default()); } #[test] From c36d14a0899c7e453e70f280e3a7762d8c77c8be Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Thu, 27 Jun 2024 09:19:10 +0200 Subject: [PATCH 13/14] Format --- pipeline/tests/powdr_std.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pipeline/tests/powdr_std.rs b/pipeline/tests/powdr_std.rs index 81e641f89..8121c9290 100644 --- a/pipeline/tests/powdr_std.rs +++ b/pipeline/tests/powdr_std.rs @@ -5,7 +5,8 @@ use powdr_number::{BigInt, Bn254Field, GoldilocksField}; use powdr_pil_analyzer::evaluator::Value; use powdr_pipeline::{ test_util::{ - evaluate_function, evaluate_integer_function, gen_estark_proof, gen_halo2_composite_proof, gen_halo2_proof, resolve_test_file, std_analyzed, test_halo2, verify_test_file + evaluate_function, evaluate_integer_function, gen_estark_proof, gen_halo2_composite_proof, + gen_halo2_proof, resolve_test_file, std_analyzed, test_halo2, verify_test_file, }, Pipeline, }; From 163b580052afc18746d364dae7929ae89a0ba7cc Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Thu, 27 Jun 2024 09:32:48 +0200 Subject: [PATCH 14/14] Fix compile error --- backend/src/plonky3/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/src/plonky3/mod.rs b/backend/src/plonky3/mod.rs index 4b1eb9867..0bd4c35b5 100644 --- a/backend/src/plonky3/mod.rs +++ b/backend/src/plonky3/mod.rs @@ -1,4 +1,4 @@ -use std::{io, path::Path}; +use std::{io, path::PathBuf}; use powdr_ast::analyzed::Analyzed; use powdr_executor::witgen::WitgenCallback; @@ -14,7 +14,7 @@ impl BackendFactory for Factory { &self, pil: &'a Analyzed, _fixed: &'a [(String, Vec)], - _output_dir: Option<&'a Path>, + _output_dir: Option, setup: Option<&mut dyn io::Read>, verification_key: Option<&mut dyn io::Read>, verification_app_key: Option<&mut dyn io::Read>,