From b94e712966ec5b95ce85d53b49be341d8ae82ba1 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 9 Apr 2024 11:06:47 +0000 Subject: [PATCH 1/9] feat: support public columns --- asm_to_pil/src/vm_to_constrained.rs | 8 +++++--- ast/src/parsed/display.rs | 7 ++++--- ast/src/parsed/mod.rs | 2 +- ast/src/parsed/visitor.rs | 8 ++++---- parser/src/powdr.lalrpop | 8 +++++--- pil_analyzer/src/statement_processor.rs | 13 +++++++++---- 6 files changed, 28 insertions(+), 18 deletions(-) diff --git a/asm_to_pil/src/vm_to_constrained.rs b/asm_to_pil/src/vm_to_constrained.rs index d944ce7d8f..9d85a785a4 100644 --- a/asm_to_pil/src/vm_to_constrained.rs +++ b/asm_to_pil/src/vm_to_constrained.rs @@ -313,7 +313,7 @@ impl ASMPILConverter { ty, }, ); - self.pil.push(witness_column(start, name, None)); + self.pil.push(witness_column(start, name, None, false)); } fn handle_instruction_def( @@ -846,7 +846,7 @@ impl ASMPILConverter { ), ) }); - witness_column(0, free_value, prover_query) + witness_column(0, free_value, prover_query, false) }) .collect::>(); self.pil.extend(free_value_pil); @@ -877,7 +877,7 @@ impl ASMPILConverter { /// Creates a pair of witness and fixed column and matches them in the lookup. fn create_witness_fixed_pair(&mut self, start: usize, name: &str) { let fixed_name = format!("p_{name}"); - self.pil.push(witness_column(start, name, None)); + self.pil.push(witness_column(start, name, None, false)); self.line_lookup .push((name.to_string(), fixed_name.clone())); self.rom_constant_names.push(fixed_name); @@ -1082,6 +1082,7 @@ fn witness_column, T>( start: usize, name: S, def: Option>, + is_public: bool ) -> PilStatement { PilStatement::PolynomialCommitDeclaration( start, @@ -1090,6 +1091,7 @@ fn witness_column, T>( array_size: None, }], def, + is_public, ) } diff --git a/ast/src/parsed/display.rs b/ast/src/parsed/display.rs index 8d4168b2f3..8aa8f803df 100644 --- a/ast/src/parsed/display.rs +++ b/ast/src/parsed/display.rs @@ -375,12 +375,13 @@ impl Display for PilStatement { PilStatement::PolynomialConstantDefinition(_, name, definition) => { write!(f, "pol constant {name}{definition};") } - PilStatement::PolynomialCommitDeclaration(_, names, value) => { + PilStatement::PolynomialCommitDeclaration(_, names, value, public) => { write!( f, - "pol commit {}{};", + "pol commit {}{}{};", + if *public { "public " } else { " " }, names.iter().format(", "), - value.as_ref().map(|v| format!("{v}")).unwrap_or_default() + value.as_ref().map(|v| format!("{v}")).unwrap_or_default(), ) } PilStatement::PolynomialIdentity(_, _attr, expression) => { diff --git a/ast/src/parsed/mod.rs b/ast/src/parsed/mod.rs index 1c6bfeafb5..83533a2b9a 100644 --- a/ast/src/parsed/mod.rs +++ b/ast/src/parsed/mod.rs @@ -33,7 +33,7 @@ pub enum PilStatement { ), PolynomialConstantDeclaration(usize, Vec>), PolynomialConstantDefinition(usize, String, FunctionDefinition), - PolynomialCommitDeclaration(usize, Vec>, Option>), + PolynomialCommitDeclaration(usize, Vec>, Option>, /*public=*/ bool), PolynomialIdentity(usize, Option, Expression), PlookupIdentity( usize, diff --git a/ast/src/parsed/visitor.rs b/ast/src/parsed/visitor.rs index 8050399a0f..52811b1626 100644 --- a/ast/src/parsed/visitor.rs +++ b/ast/src/parsed/visitor.rs @@ -218,10 +218,10 @@ impl ExpressionVisitable> for Pi .try_for_each(|e| e.visit_expressions_mut(f, o)), PilStatement::PolynomialConstantDefinition(_, _, fundef) - | PilStatement::PolynomialCommitDeclaration(_, _, Some(fundef)) => { + | PilStatement::PolynomialCommitDeclaration(_, _, Some(fundef), _) => { fundef.visit_expressions_mut(f, o) } - PilStatement::PolynomialCommitDeclaration(_, _, None) + PilStatement::PolynomialCommitDeclaration(_, _, None, _) | PilStatement::Include(_, _) | PilStatement::PolynomialConstantDeclaration(_, _) | PilStatement::LetStatement(_, _, None) => ControlFlow::Continue(()), @@ -260,10 +260,10 @@ impl ExpressionVisitable> for Pi .try_for_each(|e| e.visit_expressions(f, o)), PilStatement::PolynomialConstantDefinition(_, _, fundef) - | PilStatement::PolynomialCommitDeclaration(_, _, Some(fundef)) => { + | PilStatement::PolynomialCommitDeclaration(_, _, Some(fundef), _) => { fundef.visit_expressions(f, o) } - PilStatement::PolynomialCommitDeclaration(_, _, None) + PilStatement::PolynomialCommitDeclaration(_, _, None, _) | PilStatement::Include(_, _) | PilStatement::PolynomialConstantDeclaration(_, _) | PilStatement::LetStatement(_, _, None) => ControlFlow::Continue(()), diff --git a/parser/src/powdr.lalrpop b/parser/src/powdr.lalrpop index 763a5fd888..2b92ae376f 100644 --- a/parser/src/powdr.lalrpop +++ b/parser/src/powdr.lalrpop @@ -81,9 +81,10 @@ ConstantDefinition: PilStatement = { } PolynomialDefinition: PilStatement = { - <@L> PolCol "=" => PilStatement::PolynomialDefinition(<>) + PolCol "=" => PilStatement::PolynomialDefinition(<>) } + PublicDeclaration: PilStatement = { <@L> "public" "=" @@ -121,9 +122,10 @@ ArrayLiteralTerm: ArrayExpression = { } PolynomialCommitDeclaration: PilStatement = { - <@L> PolCol CommitWitness => PilStatement::PolynomialCommitDeclaration(<>, None), + <@L> PolCol CommitWitness => PilStatement::PolynomialCommitDeclaration(<>, None, false), + <@L> PolCol "public" => PilStatement::PolynomialCommitDeclaration(<>, None, true), PolCol CommitWitness "(" ")" "query" - => PilStatement::PolynomialCommitDeclaration(start, vec![name], Some(FunctionDefinition::Query(param, value))) + => PilStatement::PolynomialCommitDeclaration(start, vec![name], Some(FunctionDefinition::Query(param, value)), false) } PolynomialIdentity: PilStatement = { diff --git a/pil_analyzer/src/statement_processor.rs b/pil_analyzer/src/statement_processor.rs index 5bb02bbd15..ffad38831e 100644 --- a/pil_analyzer/src/statement_processor.rs +++ b/pil_analyzer/src/statement_processor.rs @@ -110,7 +110,7 @@ where self.handle_public_declaration(start, name, polynomial, array_index, index) } PilStatement::PolynomialConstantDeclaration(start, polynomials) => { - self.handle_polynomial_declarations(start, polynomials, PolynomialType::Constant) + self.handle_polynomial_declarations(start, polynomials, PolynomialType::Constant, false) } PilStatement::PolynomialConstantDefinition(start, name, definition) => self .handle_symbol_definition( @@ -120,10 +120,10 @@ where SymbolKind::Poly(PolynomialType::Constant), Some(definition), ), - PilStatement::PolynomialCommitDeclaration(start, polynomials, None) => { - self.handle_polynomial_declarations(start, polynomials, PolynomialType::Committed) + PilStatement::PolynomialCommitDeclaration(start, polynomials, None, is_public) => { + self.handle_polynomial_declarations(start, polynomials, PolynomialType::Committed, is_public) } - PilStatement::PolynomialCommitDeclaration(start, mut polynomials, Some(definition)) => { + PilStatement::PolynomialCommitDeclaration(start, mut polynomials, Some(definition), _) => { assert!(polynomials.len() == 1); let name = polynomials.pop().unwrap(); self.handle_symbol_definition( @@ -267,10 +267,15 @@ where start: usize, polynomials: Vec>, polynomial_type: PolynomialType, + is_public: bool, ) -> Vec> { polynomials .into_iter() .flat_map(|PolynomialName { name, array_size }| { + + // TODO: hack: add an is_public modifier to the end of a commited polynomial???? + let name = if is_public { format!("{name}__is_public")} else {name}; + self.handle_symbol_definition( start, name, From 31d88611552f663dd10ab548b63f7992c32c5df6 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Fri, 12 Apr 2024 12:02:50 +0000 Subject: [PATCH 2/9] feat: codegen public input columns --- asm_to_pil/src/vm_to_constrained.rs | 2 +- ast/src/parsed/mod.rs | 7 +- bberg/src/flavor_builder.rs | 7 ++ bberg/src/prover_builder.rs | 14 +++- bberg/src/verifier_builder.rs | 86 ++++++++++++++++++++++--- bberg/src/vm_builder.rs | 36 ++++++++++- parser/src/lib.rs | 3 +- pil_analyzer/src/statement_processor.rs | 34 +++++++--- 8 files changed, 163 insertions(+), 26 deletions(-) diff --git a/asm_to_pil/src/vm_to_constrained.rs b/asm_to_pil/src/vm_to_constrained.rs index 9d85a785a4..c5f837e451 100644 --- a/asm_to_pil/src/vm_to_constrained.rs +++ b/asm_to_pil/src/vm_to_constrained.rs @@ -1082,7 +1082,7 @@ fn witness_column, T>( start: usize, name: S, def: Option>, - is_public: bool + is_public: bool, ) -> PilStatement { PilStatement::PolynomialCommitDeclaration( start, diff --git a/ast/src/parsed/mod.rs b/ast/src/parsed/mod.rs index 83533a2b9a..671ba865ee 100644 --- a/ast/src/parsed/mod.rs +++ b/ast/src/parsed/mod.rs @@ -33,7 +33,12 @@ pub enum PilStatement { ), PolynomialConstantDeclaration(usize, Vec>), PolynomialConstantDefinition(usize, String, FunctionDefinition), - PolynomialCommitDeclaration(usize, Vec>, Option>, /*public=*/ bool), + PolynomialCommitDeclaration( + usize, + Vec>, + Option>, + /*public=*/ bool, + ), PolynomialIdentity(usize, Option, Expression), PlookupIdentity( usize, diff --git a/bberg/src/flavor_builder.rs b/bberg/src/flavor_builder.rs index 61c80938a4..747b28de4c 100644 --- a/bberg/src/flavor_builder.rs +++ b/bberg/src/flavor_builder.rs @@ -490,7 +490,14 @@ fn create_commitment_labels(all_ents: &[String]) -> String { ) } +/// Create the compute_logderivative_inverses function +/// +/// If we do not have any lookups, we do not need to include this round fn create_compute_logderivative_inverses(flavor_name: &str, lookups: &[String]) -> String { + if lookups.is_empty() { + return "".to_string(); + } + let compute_inverse_transformation = |lookup_name: &String| { format!("bb::compute_logderivative_inverse<{flavor_name}Flavor, {lookup_name}_relation>(prover_polynomials, relation_parameters, this->circuit_size);") }; diff --git a/bberg/src/prover_builder.rs b/bberg/src/prover_builder.rs index b7a313e2d3..61f0202e1e 100644 --- a/bberg/src/prover_builder.rs +++ b/bberg/src/prover_builder.rs @@ -15,6 +15,7 @@ pub trait ProverBuilder { impl ProverBuilder for BBFiles { fn create_prover_hpp(&mut self, name: &str) { let include_str = includes_hpp(&snake_case(name)); + let prover_hpp = format!(" {include_str} namespace bb {{ @@ -91,7 +92,16 @@ impl ProverBuilder for BBFiles { let include_str = includes_cpp(&snake_case(name)); let polynomial_commitment_phase = create_commitments_phase(commitment_polys); - let log_derivative_inverse_phase = create_log_derivative_inverse_round(lookup_names); + + let (call_log_derivative_phase, log_derivative_inverse_phase): (String, String) = + if lookup_names.is_empty() { + ("".to_owned(), "".to_owned()) + } else { + ( + "execute_log_derivative_inverse_round();".to_owned(), + create_log_derivative_inverse_round(lookup_names), + ) + }; let prover_cpp = format!(" {include_str} @@ -208,7 +218,7 @@ impl ProverBuilder for BBFiles { execute_wire_commitments_round(); // Compute sorted list accumulator and commitment - execute_log_derivative_inverse_round(); + {call_log_derivative_phase} // Fiat-Shamir: alpha // Run sumcheck subprotocol. diff --git a/bberg/src/verifier_builder.rs b/bberg/src/verifier_builder.rs index db789e18cc..305dc33e0c 100644 --- a/bberg/src/verifier_builder.rs +++ b/bberg/src/verifier_builder.rs @@ -4,13 +4,25 @@ use crate::{ }; pub trait VerifierBuilder { - fn create_verifier_cpp(&mut self, name: &str, witness: &[String], inverses: &[String]); + fn create_verifier_cpp( + &mut self, + name: &str, + witness: &[String], + inverses: &[String], + public_cols: &[String], + ); - fn create_verifier_hpp(&mut self, name: &str); + fn create_verifier_hpp(&mut self, name: &str, public_cols: &[String]); } impl VerifierBuilder for BBFiles { - fn create_verifier_cpp(&mut self, name: &str, witness: &[String], inverses: &[String]) { + fn create_verifier_cpp( + &mut self, + name: &str, + witness: &[String], + inverses: &[String], + public_cols: &[String], + ) { let include_str = includes_cpp(&snake_case(name)); let wire_transformation = |n: &String| { @@ -19,12 +31,49 @@ impl VerifierBuilder for BBFiles { ) }; let wire_commitments = map_with_newline(witness, wire_transformation); + + let has_public_input_columns = !public_cols.is_empty(); + let has_inverses = !inverses.is_empty(); + + let get_inverse_challenges = if has_inverses { + " + auto [beta, gamm] = transcript->template get_challenges(\"beta\", \"gamma\"); + relation_parameters.beta = beta; + relation_parameters.gamma = gamm; + " + .to_string() + } else { + "".to_owned() + }; + + let verify_proof_function_declaration: String = if has_public_input_columns { + format!("bool {name}Verifier::verify_proof(const HonkProof& proof, const std::vector& public_inputs)") + } else { + format!("bool {name}Verifier::verify_proof(const HonkProof& proof)") + }; + + let public_inputs_check = if has_public_input_columns { + let public_inputs_column = public_cols[0].clone(); // asserted to be 1 for the meantime, this will be generalized when required + format!( + " + FF public_column_evaluation = evaluate_public_input_column(public_inputs, multivariate_challenge); + if (public_column_evaluation != claimed_evaluations.{public_inputs_column}) {{ + return false; + }} + " + ) + } else { + "".to_owned() + }; + let inverse_commitments = map_with_newline(inverses, wire_transformation); let ver_cpp = format!(" {include_str} namespace bb {{ + + {name}Verifier::{name}Verifier(std::shared_ptr verifier_key) : key(verifier_key) {{}} @@ -41,12 +90,20 @@ impl VerifierBuilder for BBFiles { commitments.clear(); return *this; }} + + using FF = {name}Flavor::FF; + + // Evaluate the given public input column over the multivariate challenge points + [[maybe_unused]] FF evaluate_public_input_column(std::vector points, std::vector challenges) {{ + Polynomial polynomial(points); + return polynomial.evaluate_mle(challenges); + }} /** * @brief This function verifies an {name} Honk proof for given program settings. * */ - bool {name}Verifier::verify_proof(const HonkProof& proof) + {verify_proof_function_declaration} {{ using Flavor = {name}Flavor; using FF = Flavor::FF; @@ -72,9 +129,7 @@ impl VerifierBuilder for BBFiles { // Get commitments to VM wires {wire_commitments} - auto [beta, gamm] = transcript->template get_challenges(\"beta\", \"gamma\"); - relation_parameters.beta = beta; - relation_parameters.gamma = gamm; + {get_inverse_challenges} // Get commitments to inverses {inverse_commitments} @@ -97,6 +152,8 @@ impl VerifierBuilder for BBFiles { if (sumcheck_verified.has_value() && !sumcheck_verified.value()) {{ return false; }} + + {public_inputs_check} // Execute ZeroMorph rounds. See https://hackmd.io/dlf9xEwhTQyE3hiGbq4FsA?view for a complete description of the // unrolled protocol. @@ -126,8 +183,18 @@ impl VerifierBuilder for BBFiles { ); } - fn create_verifier_hpp(&mut self, name: &str) { + fn create_verifier_hpp(&mut self, name: &str, public_cols: &[String]) { let include_str = include_hpp(&snake_case(name)); + + // If there are public input columns, then the generated verifier must take them in as an argument for the verify_proof + // TODO: cleanup + let verify_proof = if !public_cols.is_empty() { + "bool verify_proof(const HonkProof& proof, const std::vector& public_inputs);" + .to_string() + } else { + "bool verify_proof(const HonkProof& proof);".to_owned() + }; + let ver_hpp = format!( " {include_str} @@ -149,7 +216,7 @@ impl VerifierBuilder for BBFiles { {name}Verifier& operator=(const {name}Verifier& other) = delete; {name}Verifier& operator=({name}Verifier&& other) noexcept; - bool verify_proof(const HonkProof& proof); + {verify_proof} std::shared_ptr key; std::map commitments; @@ -188,6 +255,7 @@ fn includes_cpp(name: &str) -> String { #include \"./{name}_verifier.hpp\" #include \"barretenberg/commitment_schemes/zeromorph/zeromorph.hpp\" #include \"barretenberg/numeric/bitop/get_msb.hpp\" + #include \"barretenberg/polynomials/polynomial.hpp\" #include \"barretenberg/transcript/transcript.hpp\" " ) diff --git a/bberg/src/vm_builder.rs b/bberg/src/vm_builder.rs index ee877a2342..64add2ef91 100644 --- a/bberg/src/vm_builder.rs +++ b/bberg/src/vm_builder.rs @@ -1,5 +1,6 @@ use ast::analyzed::Analyzed; +use itertools::Itertools; use number::FieldElement; use crate::circuit_builder::CircuitBuilder; @@ -29,6 +30,8 @@ struct ColumnGroups { fixed: Vec, /// witness or commit columns in pil -> will be found in proof witness: Vec, + // public input columns, evaluations will be calculated within the verifier + public: Vec, /// witness or commit columns in pil, with out the inverse columns witnesses_without_inverses: Vec, /// fixed + witness columns without lookup inverses @@ -89,6 +92,7 @@ pub(crate) fn analyzed_to_cpp( let ColumnGroups { fixed, witness, + public, witnesses_without_inverses, all_cols, all_cols_without_inverses, @@ -130,8 +134,8 @@ pub(crate) fn analyzed_to_cpp( bb_files.create_composer_hpp(file_name); // ----------------------- Create the Verifier files ----------------------- - bb_files.create_verifier_cpp(file_name, &witnesses_without_inverses, &inverses); - bb_files.create_verifier_hpp(file_name); + bb_files.create_verifier_cpp(file_name, &witnesses_without_inverses, &inverses, &public); + bb_files.create_verifier_hpp(file_name, &public); // ----------------------- Create the Prover files ----------------------- bb_files.create_prover_cpp(file_name, &witnesses_without_inverses, &inverses); @@ -165,7 +169,32 @@ fn get_all_col_names( // Gather sanitized column names let fixed_names = collect_col(fixed, sanitize); - let witness_names = collect_col(witness, sanitize); + + // TODO: function to separate? + let witness_names: Vec = collect_col(witness, sanitize) + .into_iter() + .map(|name| { + if name.ends_with("__is_public") { + name.strip_suffix("__is_public") + .map(|s| s.to_owned()) + .unwrap() // unwrap checked above + } else { + name + } + }) + .collect(); + let public_input_column_names: Vec = collect_col(witness, sanitize) + .into_iter() + .filter_map(|name| name.strip_suffix("__is_public").map(|s| s.to_owned())) + .collect(); + assert!( + public_input_column_names.len() == 1, + "There should only be one public input column (for now)" + ); + + println!("fixed_names: {:?}", fixed_names); + println!("wit_names: {:?}", witness_names); + println!("public_names: {:?}", public_input_column_names); let inverses = flatten(&[perm_inverses, lookup_inverses]); let witnesses_without_inverses = flatten(&[witness_names.clone(), lookup_counts.clone()]); @@ -190,6 +219,7 @@ fn get_all_col_names( ColumnGroups { fixed: fixed_names, witness: witnesses_with_inverses, + public: public_input_column_names, all_cols_without_inverses, witnesses_without_inverses, all_cols, diff --git a/parser/src/lib.rs b/parser/src/lib.rs index 340d143de9..cd258d1eab 100644 --- a/parser/src/lib.rs +++ b/parser/src/lib.rs @@ -83,7 +83,8 @@ mod test { name: "t".to_string(), array_size: None }], - None + None, + false ) ]) ); diff --git a/pil_analyzer/src/statement_processor.rs b/pil_analyzer/src/statement_processor.rs index ffad38831e..9dbf83377f 100644 --- a/pil_analyzer/src/statement_processor.rs +++ b/pil_analyzer/src/statement_processor.rs @@ -109,9 +109,13 @@ where PilStatement::PublicDeclaration(start, name, polynomial, array_index, index) => { self.handle_public_declaration(start, name, polynomial, array_index, index) } - PilStatement::PolynomialConstantDeclaration(start, polynomials) => { - self.handle_polynomial_declarations(start, polynomials, PolynomialType::Constant, false) - } + PilStatement::PolynomialConstantDeclaration(start, polynomials) => self + .handle_polynomial_declarations( + start, + polynomials, + PolynomialType::Constant, + false, + ), PilStatement::PolynomialConstantDefinition(start, name, definition) => self .handle_symbol_definition( start, @@ -120,10 +124,19 @@ where SymbolKind::Poly(PolynomialType::Constant), Some(definition), ), - PilStatement::PolynomialCommitDeclaration(start, polynomials, None, is_public) => { - self.handle_polynomial_declarations(start, polynomials, PolynomialType::Committed, is_public) - } - PilStatement::PolynomialCommitDeclaration(start, mut polynomials, Some(definition), _) => { + PilStatement::PolynomialCommitDeclaration(start, polynomials, None, is_public) => self + .handle_polynomial_declarations( + start, + polynomials, + PolynomialType::Committed, + is_public, + ), + PilStatement::PolynomialCommitDeclaration( + start, + mut polynomials, + Some(definition), + _, + ) => { assert!(polynomials.len() == 1); let name = polynomials.pop().unwrap(); self.handle_symbol_definition( @@ -272,9 +285,12 @@ where polynomials .into_iter() .flat_map(|PolynomialName { name, array_size }| { - // TODO: hack: add an is_public modifier to the end of a commited polynomial???? - let name = if is_public { format!("{name}__is_public")} else {name}; + let name = if is_public { + format!("{name}__is_public") + } else { + name + }; self.handle_symbol_definition( start, From a28fe8bf15bfcbc4176fd6eefbc7acee8cf0af55 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Fri, 12 Apr 2024 12:14:33 +0000 Subject: [PATCH 3/9] chore: cleanup --- bberg/src/vm_builder.rs | 57 ++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/bberg/src/vm_builder.rs b/bberg/src/vm_builder.rs index 64add2ef91..2eaccb9d6d 100644 --- a/bberg/src/vm_builder.rs +++ b/bberg/src/vm_builder.rs @@ -169,32 +169,8 @@ fn get_all_col_names( // Gather sanitized column names let fixed_names = collect_col(fixed, sanitize); - - // TODO: function to separate? - let witness_names: Vec = collect_col(witness, sanitize) - .into_iter() - .map(|name| { - if name.ends_with("__is_public") { - name.strip_suffix("__is_public") - .map(|s| s.to_owned()) - .unwrap() // unwrap checked above - } else { - name - } - }) - .collect(); - let public_input_column_names: Vec = collect_col(witness, sanitize) - .into_iter() - .filter_map(|name| name.strip_suffix("__is_public").map(|s| s.to_owned())) - .collect(); - assert!( - public_input_column_names.len() == 1, - "There should only be one public input column (for now)" - ); - - println!("fixed_names: {:?}", fixed_names); - println!("wit_names: {:?}", witness_names); - println!("public_names: {:?}", public_input_column_names); + let witness_names = collect_col(witness, sanitize); + let (witness_names, public_input_column_names) = extract_public_input_columns(witness_names); let inverses = flatten(&[perm_inverses, lookup_inverses]); let witnesses_without_inverses = flatten(&[witness_names.clone(), lookup_counts.clone()]); @@ -230,3 +206,32 @@ fn get_all_col_names( inverses, } } + +/// Extract public input columns +/// The compiler automatically suffixes the public input columns with "__is_public" +/// This function removes the suffix and collects the columns into their own container +pub fn extract_public_input_columns(witness_columns: Vec) -> (Vec, Vec) { + let witness_names: Vec = witness_columns + .clone() + .into_iter() + .map(|name| { + if name.ends_with("__is_public") { + name.strip_suffix("__is_public") + .map(|s| s.to_owned()) + .unwrap() // unwrap checked above + } else { + name + } + }) + .collect(); + let public_input_column_names: Vec = witness_columns + .into_iter() + .filter_map(|name| name.strip_suffix("__is_public").map(|s| s.to_owned())) + .collect(); + + assert!( + public_input_column_names.len() == 1, + "There should only be one public input column (for now)" + ); + (witness_names, public_input_column_names) +} From 9b2e78985cb82185979f6a0f48abcc1a6301dd43 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Fri, 12 Apr 2024 14:01:18 +0000 Subject: [PATCH 4/9] fix: public input cols check --- bberg/src/vm_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bberg/src/vm_builder.rs b/bberg/src/vm_builder.rs index 2eaccb9d6d..18176e2bf2 100644 --- a/bberg/src/vm_builder.rs +++ b/bberg/src/vm_builder.rs @@ -230,7 +230,7 @@ pub fn extract_public_input_columns(witness_columns: Vec) -> (Vec Date: Fri, 12 Apr 2024 14:11:53 +0000 Subject: [PATCH 5/9] fix: make rendering evaluate conditional --- bberg/src/verifier_builder.rs | 37 ++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/bberg/src/verifier_builder.rs b/bberg/src/verifier_builder.rs index 305dc33e0c..203eb84025 100644 --- a/bberg/src/verifier_builder.rs +++ b/bberg/src/verifier_builder.rs @@ -52,18 +52,32 @@ impl VerifierBuilder for BBFiles { format!("bool {name}Verifier::verify_proof(const HonkProof& proof)") }; - let public_inputs_check = if has_public_input_columns { + let (public_inputs_check, evaluate_public_inputs) = if has_public_input_columns { let public_inputs_column = public_cols[0].clone(); // asserted to be 1 for the meantime, this will be generalized when required - format!( + let inputs_check = format!( + " + FF public_column_evaluation = evaluate_public_input_column(public_inputs, multivariate_challenge); + if (public_column_evaluation != claimed_evaluations.{public_inputs_column}) {{ + return false; + }} " - FF public_column_evaluation = evaluate_public_input_column(public_inputs, multivariate_challenge); - if (public_column_evaluation != claimed_evaluations.{public_inputs_column}) {{ - return false; - }} + ); + let evaluate_public_inputs = format!( " - ) + + using FF = {name}Flavor::FF; + + // Evaluate the given public input column over the multivariate challenge points + [[maybe_unused]] FF evaluate_public_input_column(std::vector points, std::vector challenges) {{ + Polynomial polynomial(points); + return polynomial.evaluate_mle(challenges); + }} + " + ); + + (inputs_check, evaluate_public_inputs) } else { - "".to_owned() + ("".to_owned(), "".to_owned()) }; let inverse_commitments = map_with_newline(inverses, wire_transformation); @@ -91,13 +105,8 @@ impl VerifierBuilder for BBFiles { return *this; }} - using FF = {name}Flavor::FF; + {evaluate_public_inputs} - // Evaluate the given public input column over the multivariate challenge points - [[maybe_unused]] FF evaluate_public_input_column(std::vector points, std::vector challenges) {{ - Polynomial polynomial(points); - return polynomial.evaluate_mle(challenges); - }} /** * @brief This function verifies an {name} Honk proof for given program settings. From bf454c1f779c10d219213468e09b557fc0c2dfce Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 7 May 2024 18:38:43 +0000 Subject: [PATCH 6/9] temp --- bberg/src/vm_builder.rs | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/bberg/src/vm_builder.rs b/bberg/src/vm_builder.rs index 18176e2bf2..c1a696f8d3 100644 --- a/bberg/src/vm_builder.rs +++ b/bberg/src/vm_builder.rs @@ -215,18 +215,27 @@ pub fn extract_public_input_columns(witness_columns: Vec) -> (Vec = witness_columns .into_iter() - .filter_map(|name| name.strip_suffix("__is_public").map(|s| s.to_owned())) + .filter_map(|name| + if name.ends_with("__is_public") { + Some(name) + } else {None} + + // TODO: fix with above + // name.strip_suffix("__is_public").map(|s| s.to_owned()) + ) .collect(); assert!( From 7d9ec8c93ed37a5f980549f9aac32de7433080fc Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Wed, 8 May 2024 19:55:24 +0000 Subject: [PATCH 7/9] opt: convert relation checks to futures --- bberg/src/circuit_builder.rs | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/bberg/src/circuit_builder.rs b/bberg/src/circuit_builder.rs index 35eec1508b..04da4e8992 100644 --- a/bberg/src/circuit_builder.rs +++ b/bberg/src/circuit_builder.rs @@ -24,6 +24,9 @@ fn circuit_hpp_includes(name: &str, relations: &[String], permutations: &[String // AUTOGENERATED FILE #pragma once + #include + #include + #include \"barretenberg/common/constexpr_utils.hpp\" #include \"barretenberg/common/throw_or_abort.hpp\" #include \"barretenberg/ecc/curves/bn254/fr.hpp\" @@ -82,9 +85,11 @@ impl CircuitBuilder for BBFiles { |name: &String| format!("polys.{name}_shift = Polynomial(polys.{name}.shifted());"); let check_circuit_transformation = |relation_name: &String| { format!( - "if (!evaluate_relation.template operator()<{name}_vm::{relation_name}>(\"{relation_name}\", {name}_vm::get_relation_label_{relation_name})) {{ - return false; - }}", + "auto {relation_name} = [=]() {{ + return evaluate_relation.template operator()<{name}_vm::{relation_name}>(\"{relation_name}\", {name}_vm::get_relation_label_{relation_name}); + }}; + relation_futures.emplace_back(std::async(std::launch::async, {relation_name})); + ", name = name, relation_name = relation_name ) @@ -92,9 +97,11 @@ impl CircuitBuilder for BBFiles { let check_lookup_transformation = |lookup_name: &String| { let lookup_name_upper = lookup_name.to_uppercase(); format!( - "if (!evaluate_logderivative.template operator()<{lookup_name}_relation>(\"{lookup_name_upper}\")) {{ - return false; - }}" + "auto {lookup_name} = [=]() {{ + return evaluate_logderivative.template operator()<{lookup_name}_relation>(\"{lookup_name_upper}\"); + }}; + relation_futures.emplace_back(std::async(std::launch::async, {lookup_name})); + " ) }; @@ -170,10 +177,21 @@ class {name}CircuitBuilder {{ {lookup_check_closure} + // Evaluate check circuit closures as futures + std::vector> relation_futures; + {check_circuit_for_each_relation} {check_circuit_for_each_lookup} + // Wait for lookup evaluations to complete + for (auto& future : relation_futures) {{ + int result = future.get(); + if (!result) {{ + return false; + }} + }} + return true; }} From 6dc4e604d53e38b880143318e522a29e12ace200 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Thu, 9 May 2024 17:41:09 +0000 Subject: [PATCH 8/9] fix: wasm compatible futures gen --- bberg/src/circuit_builder.rs | 46 ++++++++++++++++++++++++++++++++--- bberg/src/verifier_builder.rs | 13 +++++++--- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/bberg/src/circuit_builder.rs b/bberg/src/circuit_builder.rs index 04da4e8992..595f21dd1b 100644 --- a/bberg/src/circuit_builder.rs +++ b/bberg/src/circuit_builder.rs @@ -25,7 +25,9 @@ fn circuit_hpp_includes(name: &str, relations: &[String], permutations: &[String #pragma once #include +#ifndef __wasm__ #include +#endif #include \"barretenberg/common/constexpr_utils.hpp\" #include \"barretenberg/common/throw_or_abort.hpp\" @@ -88,7 +90,6 @@ impl CircuitBuilder for BBFiles { "auto {relation_name} = [=]() {{ return evaluate_relation.template operator()<{name}_vm::{relation_name}>(\"{relation_name}\", {name}_vm::get_relation_label_{relation_name}); }}; - relation_futures.emplace_back(std::async(std::launch::async, {relation_name})); ", name = name, relation_name = relation_name @@ -100,11 +101,28 @@ impl CircuitBuilder for BBFiles { "auto {lookup_name} = [=]() {{ return evaluate_logderivative.template operator()<{lookup_name}_relation>(\"{lookup_name_upper}\"); }}; - relation_futures.emplace_back(std::async(std::launch::async, {lookup_name})); " ) }; + // When we are running natively, we want check circuit to run as futures; however, futures are not supported in wasm, so we must provide an + // alternative codepath that will execute the closures in serial + let emplace_future_transformation = |relation_name: &String| { + format!( + " + relation_futures.emplace_back(std::async(std::launch::async, {relation_name})); + " + ) + }; + + let execute_serial_transformation = |relation_name: &String| { + format!( + " + {relation_name}(); + " + ) + }; + // Apply transformations let compute_polys_assignemnt = map_with_newline(all_cols_without_inverses, compute_polys_transformation); @@ -113,6 +131,14 @@ impl CircuitBuilder for BBFiles { map_with_newline(relations, check_circuit_transformation); let check_circuit_for_each_lookup = map_with_newline(permutations, check_lookup_transformation); + + // With futures + let emplace_future_relations = map_with_newline(relations, emplace_future_transformation); + let emplace_future_lookups = map_with_newline(permutations, emplace_future_transformation); + + // With threads + let serial_relations = map_with_newline(relations, execute_serial_transformation); + let serial_lookups = map_with_newline(permutations, execute_serial_transformation); let (params, lookup_check_closure) = if !permutations.is_empty() { (get_params(), get_lookup_check_closure()) @@ -125,6 +151,7 @@ impl CircuitBuilder for BBFiles { "".to_owned() }; + let circuit_hpp = format!(" {includes} @@ -177,12 +204,18 @@ class {name}CircuitBuilder {{ {lookup_check_closure} + {check_circuit_for_each_relation} + + {check_circuit_for_each_lookup} + +#ifndef __wasm__ + // Evaluate check circuit closures as futures std::vector> relation_futures; - {check_circuit_for_each_relation} + {emplace_future_relations} + {emplace_future_lookups} - {check_circuit_for_each_lookup} // Wait for lookup evaluations to complete for (auto& future : relation_futures) {{ @@ -191,6 +224,11 @@ class {name}CircuitBuilder {{ return false; }} }} +#else + {serial_relations} + {serial_lookups} + +#endif return true; }} diff --git a/bberg/src/verifier_builder.rs b/bberg/src/verifier_builder.rs index 203eb84025..fcbc42248c 100644 --- a/bberg/src/verifier_builder.rs +++ b/bberg/src/verifier_builder.rs @@ -56,7 +56,7 @@ impl VerifierBuilder for BBFiles { let public_inputs_column = public_cols[0].clone(); // asserted to be 1 for the meantime, this will be generalized when required let inputs_check = format!( " - FF public_column_evaluation = evaluate_public_input_column(public_inputs, multivariate_challenge); + FF public_column_evaluation = evaluate_public_input_column(public_inputs, circuit_size, multivariate_challenge); if (public_column_evaluation != claimed_evaluations.{public_inputs_column}) {{ return false; }} @@ -68,8 +68,14 @@ impl VerifierBuilder for BBFiles { using FF = {name}Flavor::FF; // Evaluate the given public input column over the multivariate challenge points - [[maybe_unused]] FF evaluate_public_input_column(std::vector points, std::vector challenges) {{ - Polynomial polynomial(points); + [[maybe_unused]] inline FF evaluate_public_input_column(std::vector points, const size_t circuit_size, std::vector challenges) {{ + + // TODO: we pad the points to the circuit size in order to get the correct evaluation + // This is not efficient, and will not be valid in production + std::vector new_points(circuit_size, 0); + std::copy(points.begin(), points.end(), new_points.data()); + + Polynomial polynomial(new_points); return polynomial.evaluate_mle(challenges); }} " @@ -254,6 +260,7 @@ fn include_hpp(name: &str) -> String { #include \"barretenberg/plonk/proof_system/types/proof.hpp\" #include \"barretenberg/sumcheck/sumcheck.hpp\" #include \"barretenberg/vm/generated/{name}_flavor.hpp\" +#include \"barretenberg/vm/avm_trace/constants.hpp\" " ) } From 36878230230a1ba7824fb2f3f2224bf964ce8676 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Mon, 13 May 2024 10:54:59 +0000 Subject: [PATCH 9/9] fix --- parser/src/powdr.lalrpop | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parser/src/powdr.lalrpop b/parser/src/powdr.lalrpop index 2b92ae376f..a32d718186 100644 --- a/parser/src/powdr.lalrpop +++ b/parser/src/powdr.lalrpop @@ -81,7 +81,7 @@ ConstantDefinition: PilStatement = { } PolynomialDefinition: PilStatement = { - PolCol "=" => PilStatement::PolynomialDefinition(<>) + <@L> PolCol "=" => PilStatement::PolynomialDefinition(<>) }