From 980e2cc4b1adec5e21c54fb0df8c166b0a9ca689 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Fri, 17 Feb 2023 16:58:35 -0500 Subject: [PATCH 1/3] initial commit changing log directive to accept traces and move away from printing when solving directives --- acir/src/circuit/directives.rs | 40 ++++++++++++++++++----------- acir/src/circuit/opcodes.rs | 28 ++++++++++++-------- acvm/src/lib.rs | 16 ++++++++---- acvm/src/pwg/directives.rs | 47 ++++++++++++++++++++-------------- 4 files changed, 82 insertions(+), 49 deletions(-) diff --git a/acir/src/circuit/directives.rs b/acir/src/circuit/directives.rs index f9550f88f..1efaca0d7 100644 --- a/acir/src/circuit/directives.rs +++ b/acir/src/circuit/directives.rs @@ -58,7 +58,10 @@ pub enum Directive { bits: Vec, // control bits of the network which permutes the inputs into its sorted version sort_by: Vec, // specify primary index to sort by, then the secondary,... For instance, if tuple is 2 and sort_by is [1,0], then a=[(a0,b0),..] is sorted by bi and then ai. }, - Log(LogInfo), + Log { + is_trace: bool, // This field states whether the log should be further manipulated or simply displayed to standard output + output_info: LogOutputInfo, + }, } impl Directive { @@ -159,17 +162,23 @@ impl Directive { write_u32(&mut writer, *i)?; } } - Directive::Log(info) => match info { - LogInfo::FinalizedOutput(output_string) => { - write_bytes(&mut writer, output_string.as_bytes())?; - } - LogInfo::WitnessOutput(witnesses) => { - write_u32(&mut writer, witnesses.len() as u32)?; - for w in witnesses { - write_u32(&mut writer, w.witness_index())?; + Directive::Log { + is_trace, + output_info, + } => { + write_u32(&mut writer, *is_trace as u32)?; + match output_info { + LogOutputInfo::FinalizedOutput(output_string) => { + write_bytes(&mut writer, output_string.as_bytes())?; + } + LogOutputInfo::WitnessOutput(witnesses) => { + write_u32(&mut writer, witnesses.len() as u32)?; + for w in witnesses { + write_u32(&mut writer, w.witness_index())?; + } } } - }, + } }; Ok(()) @@ -274,11 +283,12 @@ impl Directive { } #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -// If values are compile time and/or known during -// evaluation, we can form an output string during ACIR generation. -// Otherwise, we must store witnesses whose values will -// be fetched during the PWG stage. -pub enum LogInfo { +/// This info is used when solving the initial witness +/// If values are compile time and/or known during +/// evaluation, we can form an output string during ACIR generation. +/// Otherwise, we must store witnesses whose values will +/// be fetched during the PWG stage. +pub enum LogOutputInfo { FinalizedOutput(String), WitnessOutput(Vec), } diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index 674307909..b0b0aef2a 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -1,6 +1,6 @@ use std::io::{Read, Write}; -use super::directives::{Directive, LogInfo}; +use super::directives::{Directive, LogOutputInfo}; use crate::native_types::{Expression, Witness}; use crate::serialization::{read_n, read_u16, read_u32, write_bytes, write_u16, write_u32}; use crate::BlackBoxFunc; @@ -188,15 +188,23 @@ impl std::fmt::Display for Opcode { bits.last().unwrap().witness_index(), ) } - Opcode::Directive(Directive::Log(info)) => match info { - LogInfo::FinalizedOutput(output_string) => write!(f, "Log: {output_string}"), - LogInfo::WitnessOutput(witnesses) => write!( - f, - "Log: _{}..._{}", - witnesses.first().unwrap().witness_index(), - witnesses.last().unwrap().witness_index() - ), - }, + Opcode::Directive(Directive::Log { + is_trace, + output_info, + }) => { + let is_trace_display = if *is_trace { "trace" } else { "println" }; + match output_info { + LogOutputInfo::FinalizedOutput(output_string) => { + write!(f, "Log: {output_string}, log type {is_trace_display}") + } + LogOutputInfo::WitnessOutput(witnesses) => write!( + f, + "Log: _{}..._{}, log type: {is_trace_display}", + witnesses.first().unwrap().witness_index(), + witnesses.last().unwrap().witness_index(), + ), + } + } } } } diff --git a/acvm/src/lib.rs b/acvm/src/lib.rs index 9aab75b2a..122ca0cb8 100644 --- a/acvm/src/lib.rs +++ b/acvm/src/lib.rs @@ -25,7 +25,7 @@ pub use acir::FieldElement; // TODO: ExpressionHasTooManyUnknowns is specific for arithmetic expressions // TODO: we could have a error enum for arithmetic failure cases in that module // TODO that can be converted into an OpcodeNotSolvable or OpcodeResolutionError enum -#[derive(PartialEq, Eq, Debug, Error)] +#[derive(PartialEq, Eq, Debug, Error, Clone)] pub enum OpcodeNotSolvable { #[error("missing assignment for witness index {0}")] MissingAssignment(u32), @@ -35,7 +35,7 @@ pub enum OpcodeNotSolvable { UnreachableCode, } -#[derive(PartialEq, Eq, Debug, Error)] +#[derive(PartialEq, Eq, Debug, Error, Clone)] pub enum OpcodeResolutionError { #[error("cannot solve opcode: {0}")] OpcodeNotSolvable(OpcodeNotSolvable), @@ -59,6 +59,7 @@ pub trait PartialWitnessGenerator { &self, initial_witness: &mut BTreeMap, opcodes: Vec, + logs: &mut Vec, ) -> Result<(), OpcodeResolutionError> { if opcodes.is_empty() { return Ok(()); @@ -71,7 +72,12 @@ pub trait PartialWitnessGenerator { Opcode::BlackBoxFuncCall(bb_func) => { Self::solve_black_box_function_call(initial_witness, bb_func) } - Opcode::Directive(directive) => Self::solve_directives(initial_witness, directive), + Opcode::Directive(directive) => Self::solve_directives(initial_witness, directive) + .map(|possible_log| { + if let Some(solved_log) = possible_log { + logs.push(solved_log) + } + }), }; match resolution { @@ -87,7 +93,7 @@ pub trait PartialWitnessGenerator { Err(err) => return Err(err), } } - self.solve(initial_witness, unsolved_opcodes) + self.solve(initial_witness, unsolved_opcodes, logs) } fn solve_black_box_function_call( @@ -112,7 +118,7 @@ pub trait PartialWitnessGenerator { fn solve_directives( initial_witness: &mut BTreeMap, directive: &Directive, - ) -> Result<(), OpcodeResolutionError> { + ) -> Result, OpcodeResolutionError> { pwg::directives::solve_directives(initial_witness, directive) } } diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index d2b9a33e9..0c3b6d34c 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -1,7 +1,7 @@ use std::{cmp::Ordering, collections::BTreeMap}; use acir::{ - circuit::directives::{Directive, LogInfo}, + circuit::directives::{Directive, LogOutputInfo}, native_types::Witness, FieldElement, }; @@ -15,13 +15,13 @@ use super::{get_value, insert_value, sorting::route, witness_to_value}; pub fn solve_directives( initial_witness: &mut BTreeMap, directive: &Directive, -) -> Result<(), OpcodeResolutionError> { +) -> Result, OpcodeResolutionError> { match directive { Directive::Invert { x, result } => { let val = witness_to_value(initial_witness, *x)?; let inverse = val.inverse(); initial_witness.insert(*result, inverse); - Ok(()) + Ok(None) } Directive::Quotient { a, @@ -60,7 +60,7 @@ pub fn solve_directives( initial_witness, )?; - Ok(()) + Ok(None) } Directive::Truncate { a, b, c, bit_size } => { let val_a = get_value(a, initial_witness)?; @@ -82,7 +82,7 @@ pub fn solve_directives( initial_witness, )?; - Ok(()) + Ok(None) } Directive::ToRadix { a, @@ -138,7 +138,7 @@ pub fn solve_directives( } } - Ok(()) + Ok(None) } Directive::OddRange { a, b, r, bit_size } => { let val_a = witness_to_value(initial_witness, *a)?; @@ -164,7 +164,7 @@ pub fn solve_directives( initial_witness, )?; - Ok(()) + Ok(None) } Directive::PermutationSort { inputs: a, @@ -206,23 +206,30 @@ pub fn solve_directives( }; insert_witness(*w, value, initial_witness)?; } - Ok(()) + Ok(None) } - Directive::Log(info) => { - let witnesses = match info { - LogInfo::FinalizedOutput(output_string) => { - println!("{output_string}"); - return Ok(()); + Directive::Log { + is_trace, + output_info, + } => { + let witnesses = match output_info { + LogOutputInfo::FinalizedOutput(_) => { + return Ok(Some(directive.clone())); } - LogInfo::WitnessOutput(witnesses) => witnesses, + LogOutputInfo::WitnessOutput(witnesses) => witnesses, }; if witnesses.len() == 1 { + dbg!(witnesses.clone()); + let witness = &witnesses[0]; let log_value = witness_to_value(initial_witness, *witness)?; - println!("{}", format_field_string(*log_value)); - return Ok(()); + let solved_log_directive = Directive::Log { + is_trace: *is_trace, + output_info: LogOutputInfo::FinalizedOutput(format_field_string(*log_value)), + }; + return Ok(Some(solved_log_directive)); } // If multiple witnesses are to be fetched for a log directive, @@ -241,9 +248,11 @@ pub fn solve_directives( let output_witnesses_string = "[".to_owned() + &comma_separated_elements + "]"; - println!("{output_witnesses_string}"); - - Ok(()) + let solved_log_directive = Directive::Log { + is_trace: *is_trace, + output_info: LogOutputInfo::FinalizedOutput(output_witnesses_string), + }; + Ok(Some(solved_log_directive)) } } } From f7b98fa3d5d9927a6323177c9fc877ab08784cb0 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 22 Feb 2023 18:49:35 +0000 Subject: [PATCH 2/3] chore: add comment on the assumption in printing witnesses --- acir/src/circuit/opcodes.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index f3bd3013d..8e23c582d 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -172,6 +172,8 @@ impl std::fmt::Display for Opcode { LogOutputInfo::FinalizedOutput(output_string) => { write!(f, "Log: {output_string}, log type {is_trace_display}") } + // Note: This assumes that the witnesses have contiguous indices. + // This may not always be the case however we don't want to print out the full vec. LogOutputInfo::WitnessOutput(witnesses) => write!( f, "Log: _{}..._{}, log type: {is_trace_display}", From a7227ee12ca918de68bc20ec9cc60add67c5e65c Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Wed, 22 Feb 2023 18:50:16 +0000 Subject: [PATCH 3/3] chore: remove dbg! call --- acvm/src/pwg/directives.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 37de47211..44e60f885 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -197,8 +197,6 @@ pub fn solve_directives( }; if witnesses.len() == 1 { - dbg!(witnesses.clone()); - let witness = &witnesses[0]; let log_value = witness_to_value(initial_witness, *witness)?;