From d64374700353df8b864ee0b1b28b72b36a7202c6 Mon Sep 17 00:00:00 2001 From: Ben Wilson Date: Fri, 21 Mar 2025 14:09:36 +1100 Subject: [PATCH 1/2] Map all ACIR witnesses to R1CS witnesses; add some debug pretty printing --- noir-r1cs/src/compiler.rs | 25 ++++++++++-- noir-r1cs/src/main.rs | 70 ++++++++++------------------------ noir-r1cs/src/sparse_matrix.rs | 26 +++++++++++++ 3 files changed, 68 insertions(+), 53 deletions(-) diff --git a/noir-r1cs/src/compiler.rs b/noir-r1cs/src/compiler.rs index ace0ce442..b7ee62093 100644 --- a/noir-r1cs/src/compiler.rs +++ b/noir-r1cs/src/compiler.rs @@ -116,13 +116,33 @@ impl R1CS { Ok(()) } + /// Pretty print the R1CS matrices and the ACIR -> R1CS witness map, useful for debugging. + pub fn pretty_print(&self) { + if std::cmp::max(self.constraints, self.witnesses) > 15 { + println!("Matrices too large to print"); + return; + } + println!("ACIR witness <-> R1CS witness mapping:"); + for (k, v) in &self.remap { + println!("{k} <-> {v}"); + } + println!("Matrix A:"); + self.a.pretty_print(); + println!("Matrix B:"); + self.b.pretty_print(); + println!("Matrix C:"); + self.c.pretty_print(); + } + pub fn add_circuit(&mut self, circuit: &Circuit) { for opcode in circuit.opcodes.iter() { match opcode { Opcode::AssertZero(expr) => self.add_assert_zero(expr), - // TODO: Brillig is a VM used to generate witness values. It does not produce - // constraints. + // Brillig instructions are used by the ACVM to solve for ACIR witness values. + // Corresponding ACIR constraints are by Noir as AssertZeros, and we map all ACIR + // witness values to R1CS witness values, so we can safely ignore + // Opcode::BrilligCall. Opcode::BrilligCall { .. } => { println!("BrilligCall {:?}", opcode) } @@ -160,7 +180,6 @@ impl R1CS { } } } - println!("self.constraints: {:?}", self.constraints); } /// Index of the constant one witness diff --git a/noir-r1cs/src/main.rs b/noir-r1cs/src/main.rs index c48600b8d..3430a38cb 100644 --- a/noir-r1cs/src/main.rs +++ b/noir-r1cs/src/main.rs @@ -10,7 +10,7 @@ use { argh::FromArgs, noirc_artifacts::program::ProgramArtifact, rand::Rng, - std::{fs::File, iter::zip, path::PathBuf, vec}, + std::{fs::File, path::PathBuf, vec}, tracing::{info, level_filters::LevelFilter}, tracing_subscriber::{self, fmt::format::FmtSpan, EnvFilter}, utils::{file_io::deserialize_witness_stack, PrintAbi}, @@ -72,9 +72,11 @@ fn noir(args: NoirCmd) -> AnyResult<()> { "Program must have one entry point." ); let main = &program.bytecode.functions[0]; + let num_public_parameters = main.public_parameters.0.len(); + let num_acir_witnesses = main.current_witness_index as usize; info!( "ACIR: {} witnesses, {} opcodes.", - main.current_witness_index, + num_acir_witnesses, main.opcodes.len() ); @@ -85,65 +87,30 @@ fn noir(args: NoirCmd) -> AnyResult<()> { let witness_stack = witness_stack.pop().unwrap().witness; + if num_acir_witnesses < 15 { + println!("ACIR witness values:"); + (0..num_acir_witnesses).for_each(|i| { + println!("{}: {:?}", i, witness_stack[&Witness(i as u32)]); + }); + } + // Create the R1CS relation let mut r1cs = R1CS::new(); r1cs.add_circuit(main); - - // just checking the private inputs for now - let mut private_inputs_original_witnesses = vec![]; - let mut public_inputs_original_witnesses = vec![]; - - // Collect inputs and outputs - let public_inputs = main - .public_parameters - .0 - .iter() - .map(|w| { - public_inputs_original_witnesses.push(w); - r1cs.map_witness(*w) - }) - .collect::>(); - let public_outputs = main - .return_values - .0 - .iter() - .map(|w| r1cs.map_witness(*w)) - .collect::>(); - let private_inputs = main - .private_parameters - .iter() - .map(|w| { - private_inputs_original_witnesses.push(w); - r1cs.map_witness(*w) - }) - .collect::>(); + r1cs.pretty_print(); info!( "R1CS: {} witnesses, {} constraints", r1cs.witnesses, r1cs.constraints ); - // dbg!(&r1cs); - dbg!(&public_inputs); - dbg!(&public_outputs); - dbg!(&private_inputs); // Compute a satisfying witness let mut witness = vec![None; r1cs.witnesses]; witness[0] = Some(FieldElement::one()); // Constant - // Inputs - for (witness_idx, original_witness_idx) in private_inputs - .iter() - .zip(private_inputs_original_witnesses.iter()) - { - witness[*witness_idx] = Some(witness_stack[original_witness_idx]) - } - - for (witness_idx, original_witness_idx) in public_inputs - .iter() - .zip(public_inputs_original_witnesses.iter()) - { - witness[*witness_idx] = Some(witness_stack[original_witness_idx]) + // Fill in R1CS witness values with the pre-computed ACIR witness values + for (acir_witness_idx, witness_idx) in &r1cs.remap { + witness[*witness_idx] = Some(witness_stack[&Witness(*acir_witness_idx as u32)]); } // Solve constraints (this is how Noir expects it to be done, judging from ACVM) @@ -158,7 +125,10 @@ fn noir(args: NoirCmd) -> AnyResult<()> { (Some(a), Some(b), None) => (a * b, &r1cs.c), (Some(a), None, Some(c)) => (c / a, &r1cs.b), (None, Some(b), Some(c)) => (c / b, &r1cs.a), - _ => panic!("Can not solve constraint {row}."), + _ => { + dbg!(a, b, c); + panic!("Can not solve constraint {row}.") + }, }; let Some((col, val)) = solve_dot(mat.iter_row(row), &witness, val) else { panic!("Could not solve constraint {row}.") @@ -208,7 +178,7 @@ fn noir(args: NoirCmd) -> AnyResult<()> { // dbg!(&b); // dbg!(&c); - r1cs.write_json_to_file(public_inputs.len(), &witness, "r1cs.json")?; + r1cs.write_json_to_file(num_public_parameters, &witness, "r1cs.json")?; Ok(()) } diff --git a/noir-r1cs/src/sparse_matrix.rs b/noir-r1cs/src/sparse_matrix.rs index 7a2a8ee11..16f89602c 100644 --- a/noir-r1cs/src/sparse_matrix.rs +++ b/noir-r1cs/src/sparse_matrix.rs @@ -43,6 +43,32 @@ impl SparseMatrix { assert!(col < self.cols, "column index out of bounds"); self.entries.insert((row, col), value); } + + /// Return a dense representation of the matrix. + fn to_dense(&self) -> Vec> + where + F: Clone, + { + let mut result = vec![vec![self.default.clone(); self.cols]; self.rows]; + for ((i, j), value) in self.entries.iter() { + result[*i][*j] = value.clone(); + } + result + } + + /// Pretty print a dense representation of the matrix. + pub fn pretty_print(&self) + where + F: std::fmt::Debug, F: Clone, + { + let dense = self.to_dense(); + for row in dense.iter() { + for value in row.iter() { + print!("{:?}\t", value); + } + println!(); + } + } } impl SparseMatrix { From 68c53101c7e701bac2f77092bba4ff7bc92af438 Mon Sep 17 00:00:00 2001 From: Ben Wilson Date: Mon, 24 Mar 2025 10:38:49 +1100 Subject: [PATCH 2/2] pretty_print -> Display; clarifying comments --- noir-r1cs/src/compiler.rs | 38 ++++++++++++++++++---------------- noir-r1cs/src/main.rs | 2 +- noir-r1cs/src/sparse_matrix.rs | 18 ++++++++-------- 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/noir-r1cs/src/compiler.rs b/noir-r1cs/src/compiler.rs index b7ee62093..7a88d87c9 100644 --- a/noir-r1cs/src/compiler.rs +++ b/noir-r1cs/src/compiler.rs @@ -116,24 +116,6 @@ impl R1CS { Ok(()) } - /// Pretty print the R1CS matrices and the ACIR -> R1CS witness map, useful for debugging. - pub fn pretty_print(&self) { - if std::cmp::max(self.constraints, self.witnesses) > 15 { - println!("Matrices too large to print"); - return; - } - println!("ACIR witness <-> R1CS witness mapping:"); - for (k, v) in &self.remap { - println!("{k} <-> {v}"); - } - println!("Matrix A:"); - self.a.pretty_print(); - println!("Matrix B:"); - self.b.pretty_print(); - println!("Matrix C:"); - self.c.pretty_print(); - } - pub fn add_circuit(&mut self, circuit: &Circuit) { for opcode in circuit.opcodes.iter() { match opcode { @@ -287,3 +269,23 @@ impl R1CS { self.add_constraint(&a, &b, &linear); } } + +/// Print the R1CS matrices and the ACIR -> R1CS witness map, useful for debugging. +impl std::fmt::Display for R1CS { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + if std::cmp::max(self.constraints, self.witnesses) > 15 { + println!("Matrices too large to print"); + return Ok(()); + } + writeln!(f, "ACIR witness <-> R1CS witness mapping:")?; + for (k, v) in &self.remap { + writeln!(f, "{k} <-> {v}")?; + } + writeln!(f, "Matrix A:")?; + write!(f, "{}", self.a)?; + writeln!(f, "Matrix B:")?; + write!(f, "{}", self.b)?; + writeln!(f, "Matrix C:")?; + write!(f, "{}", self.c) + } +} diff --git a/noir-r1cs/src/main.rs b/noir-r1cs/src/main.rs index 3430a38cb..d71fba730 100644 --- a/noir-r1cs/src/main.rs +++ b/noir-r1cs/src/main.rs @@ -97,7 +97,7 @@ fn noir(args: NoirCmd) -> AnyResult<()> { // Create the R1CS relation let mut r1cs = R1CS::new(); r1cs.add_circuit(main); - r1cs.pretty_print(); + print!("{}", r1cs); info!( "R1CS: {} witnesses, {} constraints", diff --git a/noir-r1cs/src/sparse_matrix.rs b/noir-r1cs/src/sparse_matrix.rs index 16f89602c..d00a5f5bc 100644 --- a/noir-r1cs/src/sparse_matrix.rs +++ b/noir-r1cs/src/sparse_matrix.rs @@ -1,6 +1,5 @@ use std::{ - collections::BTreeMap, - ops::{Add, AddAssign, Index, IndexMut, Mul}, + collections::BTreeMap, fmt::{Debug, Display, Formatter}, ops::{Add, AddAssign, Index, IndexMut, Mul} }; /// A sparse matrix with elements of type `F`. @@ -45,6 +44,7 @@ impl SparseMatrix { } /// Return a dense representation of the matrix. + /// (This is a helper method for debugging.) fn to_dense(&self) -> Vec> where F: Clone, @@ -55,19 +55,19 @@ impl SparseMatrix { } result } +} - /// Pretty print a dense representation of the matrix. - pub fn pretty_print(&self) - where - F: std::fmt::Debug, F: Clone, - { +/// Print a dense representation of the matrix, for debugging. +impl Display for SparseMatrix { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { let dense = self.to_dense(); for row in dense.iter() { for value in row.iter() { - print!("{:?}\t", value); + write!(f, "{:?}\t", value)?; } - println!(); + writeln!(f)?; } + Ok(()) } }