From c1577fe22ca2fe64c11d67cb1f80055f5bb4f0b5 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Thu, 19 Jan 2023 23:34:37 -0500 Subject: [PATCH 01/12] log directive and logic for solving it --- acir/src/circuit/directives.rs | 25 +++++++++++++++++- acir/src/circuit/opcodes.rs | 23 ++++++++++++++++- acvm/src/pwg/directives.rs | 46 +++++++++++++++++++++++++++++++++- 3 files changed, 91 insertions(+), 3 deletions(-) diff --git a/acir/src/circuit/directives.rs b/acir/src/circuit/directives.rs index c3d8e2cd0..d1e4e6c55 100644 --- a/acir/src/circuit/directives.rs +++ b/acir/src/circuit/directives.rs @@ -2,7 +2,7 @@ use std::io::{Read, Write}; use crate::{ native_types::{Expression, Witness}, - serialisation::{read_n, read_u16, read_u32, write_bytes, write_u16, write_u32}, + serialisation::{read_n, read_u16, read_u32, write_bytes, write_n, write_u16, write_u32}, }; use serde::{Deserialize, Serialize}; @@ -48,6 +48,8 @@ pub enum Directive { b: Vec, radix: u32, }, + + Log(LogInfo), } impl Directive { @@ -58,6 +60,7 @@ impl Directive { Directive::Truncate { .. } => "truncate", Directive::OddRange { .. } => "odd_range", Directive::ToRadix { .. } => "to_radix", + Directive::Log { .. } => "log", } } fn to_u16(&self) -> u16 { @@ -67,6 +70,7 @@ impl Directive { Directive::Truncate { .. } => 2, Directive::OddRange { .. } => 3, Directive::ToRadix { .. } => 4, + Directive::Log { .. } => 5, } } @@ -116,6 +120,17 @@ impl Directive { } write_u32(&mut writer, *radix)?; } + 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())?; + } + } + }, }; Ok(()) @@ -184,6 +199,14 @@ 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 gen. +// Otherwise, we must store witnesses whose values will be fetched during the PWG. +pub enum LogInfo { + FinalizedOutput(String), + WitnessOutput(Vec), +} + #[test] fn serialisation_roundtrip() { fn read_write(directive: Directive) -> (Directive, Directive) { diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index 37017adde..e9f72bde9 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; +use super::directives::{Directive, LogInfo}; use crate::native_types::{Expression, Witness}; use crate::serialisation::{read_n, read_u16, read_u32, write_bytes, write_u16, write_u32}; use crate::BlackBoxFunc; @@ -164,6 +164,27 @@ impl std::fmt::Display for Opcode { b.last().unwrap().witness_index(), ) } + // Opcode::Directive(Directive::Log { output_string, witnesses }) => { + // if !witnesses.is_empty() { + // write!( + // f, + // "Log: _{}..._{}", + // witnesses.first().unwrap().witness_index(), + // witnesses.last().unwrap().witness_index() + // ) + // } else { + // write!(f, "Log: {}", output_string) + // } + // } + 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() + ), + }, } } } diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 94bc2d60b..f79208016 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -1,6 +1,10 @@ use std::collections::BTreeMap; -use acir::{circuit::directives::Directive, native_types::Witness, FieldElement}; +use acir::{ + circuit::directives::{Directive, LogInfo}, + native_types::Witness, + FieldElement, +}; use num_bigint::BigUint; use num_traits::{One, Zero}; @@ -110,5 +114,45 @@ pub fn solve_directives( Ok(()) } + Directive::Log(info) => { + match info { + LogInfo::FinalizedOutput(output_string) => println!("{}", output_string), + LogInfo::WitnessOutput(witnesses) => { + if witnesses.len() == 1 { + match initial_witness.entry(witnesses[0]) { + std::collections::btree_map::Entry::Vacant(_) => { + unreachable!("log entry does must have a witness"); + } + std::collections::btree_map::Entry::Occupied(e) => { + println!("{}", e.get().to_hex()); + } + } + } else { + // If multiple witnesses are to be fetched for a log directive, + // it assumed that an array is meant to be printed to standard output + let mut output_witnesses_string = "".to_owned(); + output_witnesses_string.push_str("["); + let mut iter = witnesses.iter().peekable(); + while let Some(w) = iter.next() { + let elem = match initial_witness.entry(*w) { + std::collections::btree_map::Entry::Vacant(_) => { + // TODO switch this to an error + unreachable!("log entry does must have a witness"); + } + std::collections::btree_map::Entry::Occupied(e) => e.get().clone(), + }; + if iter.peek().is_none() { + output_witnesses_string.push_str(&format!("{}", elem.to_hex())); + } else { + output_witnesses_string.push_str(&format!("{}, ", elem.to_hex())); + } + } + output_witnesses_string.push_str("]"); + println!("{}", output_witnesses_string); + } + } + } + Ok(()) + } } } From a048a0c589fc43322f2449c716acaf4e52deae39 Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 23 Jan 2023 16:49:39 -0500 Subject: [PATCH 02/12] switch unreachable to error OpcodeNotSolvable --- acvm/src/pwg/directives.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index f79208016..effe1982d 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -8,7 +8,7 @@ use acir::{ use num_bigint::BigUint; use num_traits::{One, Zero}; -use crate::OpcodeResolutionError; +use crate::{OpcodeNotSolvable, OpcodeResolutionError}; use super::{get_value, witness_to_value}; @@ -136,8 +136,9 @@ pub fn solve_directives( while let Some(w) = iter.next() { let elem = match initial_witness.entry(*w) { std::collections::btree_map::Entry::Vacant(_) => { - // TODO switch this to an error - unreachable!("log entry does must have a witness"); + return Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::MissingAssignment(w.0), + )) } std::collections::btree_map::Entry::Occupied(e) => e.get().clone(), }; From a358bd0ad0247ae3f5df99b10984d6ab9110d14d Mon Sep 17 00:00:00 2001 From: vezenovm Date: Tue, 24 Jan 2023 11:26:44 -0500 Subject: [PATCH 03/12] remove unnecessary comment --- acir/src/circuit/opcodes.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index e9f72bde9..d1e53926e 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -164,18 +164,6 @@ impl std::fmt::Display for Opcode { b.last().unwrap().witness_index(), ) } - // Opcode::Directive(Directive::Log { output_string, witnesses }) => { - // if !witnesses.is_empty() { - // write!( - // f, - // "Log: _{}..._{}", - // witnesses.first().unwrap().witness_index(), - // witnesses.last().unwrap().witness_index() - // ) - // } else { - // write!(f, "Log: {}", output_string) - // } - // } Opcode::Directive(Directive::Log(info)) => match info { LogInfo::FinalizedOutput(output_string) => write!(f, "Log: {}", output_string), LogInfo::WitnessOutput(witnesses) => write!( From 15e8d912759befcf3f744ab257f0eb40cc9a49ea Mon Sep 17 00:00:00 2001 From: vezenovm Date: Mon, 30 Jan 2023 13:33:51 -0500 Subject: [PATCH 04/12] some cargo clippy fixes --- acir/src/circuit/directives.rs | 2 +- acir/src/circuit/opcodes.rs | 2 +- acvm/src/lib.rs | 5 +---- acvm/src/pwg/directives.rs | 12 ++++++------ 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/acir/src/circuit/directives.rs b/acir/src/circuit/directives.rs index d1e4e6c55..b7b91cdbc 100644 --- a/acir/src/circuit/directives.rs +++ b/acir/src/circuit/directives.rs @@ -2,7 +2,7 @@ use std::io::{Read, Write}; use crate::{ native_types::{Expression, Witness}, - serialisation::{read_n, read_u16, read_u32, write_bytes, write_n, write_u16, write_u32}, + serialisation::{read_n, read_u16, read_u32, write_bytes, write_u16, write_u32}, }; use serde::{Deserialize, Serialize}; diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index d1e53926e..a253be75b 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -165,7 +165,7 @@ impl std::fmt::Display for Opcode { ) } Opcode::Directive(Directive::Log(info)) => match info { - LogInfo::FinalizedOutput(output_string) => write!(f, "Log: {}", output_string), + LogInfo::FinalizedOutput(output_string) => write!(f, "Log: {output_string}"), LogInfo::WitnessOutput(witnesses) => write!( f, "Log: _{}..._{}", diff --git a/acvm/src/lib.rs b/acvm/src/lib.rs index c44fd9632..1e69fce1a 100644 --- a/acvm/src/lib.rs +++ b/acvm/src/lib.rs @@ -211,10 +211,7 @@ pub fn default_is_blackbox_supported( // attempt to transform into supported gates. If these are also not available // then a compiler error will be emitted. fn plonk_is_supported(opcode: &BlackBoxFunc) -> bool { - match opcode { - BlackBoxFunc::AES => false, - _ => true, - } + !matches!(opcode, BlackBoxFunc::AES) } match language { diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index effe1982d..50e064a92 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -116,7 +116,7 @@ pub fn solve_directives( } Directive::Log(info) => { match info { - LogInfo::FinalizedOutput(output_string) => println!("{}", output_string), + LogInfo::FinalizedOutput(output_string) => println!("{output_string}"), LogInfo::WitnessOutput(witnesses) => { if witnesses.len() == 1 { match initial_witness.entry(witnesses[0]) { @@ -131,7 +131,7 @@ pub fn solve_directives( // If multiple witnesses are to be fetched for a log directive, // it assumed that an array is meant to be printed to standard output let mut output_witnesses_string = "".to_owned(); - output_witnesses_string.push_str("["); + output_witnesses_string.push('['); let mut iter = witnesses.iter().peekable(); while let Some(w) = iter.next() { let elem = match initial_witness.entry(*w) { @@ -140,16 +140,16 @@ pub fn solve_directives( OpcodeNotSolvable::MissingAssignment(w.0), )) } - std::collections::btree_map::Entry::Occupied(e) => e.get().clone(), + std::collections::btree_map::Entry::Occupied(e) => *e.get(), }; if iter.peek().is_none() { - output_witnesses_string.push_str(&format!("{}", elem.to_hex())); + output_witnesses_string.push_str(&elem.to_hex()); } else { output_witnesses_string.push_str(&format!("{}, ", elem.to_hex())); } } - output_witnesses_string.push_str("]"); - println!("{}", output_witnesses_string); + output_witnesses_string.push(']'); + println!("{output_witnesses_string}"); } } } From 8461facb98f35a81e2b154a53cf5ca23b45133bd Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Fri, 3 Feb 2023 23:23:02 +0000 Subject: [PATCH 05/12] decrease rightward shift --- acvm/src/pwg/directives.rs | 76 ++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 50e064a92..9644949bc 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -115,44 +115,50 @@ pub fn solve_directives( Ok(()) } Directive::Log(info) => { - match info { - LogInfo::FinalizedOutput(output_string) => println!("{output_string}"), - LogInfo::WitnessOutput(witnesses) => { - if witnesses.len() == 1 { - match initial_witness.entry(witnesses[0]) { - std::collections::btree_map::Entry::Vacant(_) => { - unreachable!("log entry does must have a witness"); - } - std::collections::btree_map::Entry::Occupied(e) => { - println!("{}", e.get().to_hex()); - } - } - } else { - // If multiple witnesses are to be fetched for a log directive, - // it assumed that an array is meant to be printed to standard output - let mut output_witnesses_string = "".to_owned(); - output_witnesses_string.push('['); - let mut iter = witnesses.iter().peekable(); - while let Some(w) = iter.next() { - let elem = match initial_witness.entry(*w) { - std::collections::btree_map::Entry::Vacant(_) => { - return Err(OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::MissingAssignment(w.0), - )) - } - std::collections::btree_map::Entry::Occupied(e) => *e.get(), - }; - if iter.peek().is_none() { - output_witnesses_string.push_str(&elem.to_hex()); - } else { - output_witnesses_string.push_str(&format!("{}, ", elem.to_hex())); - } - } - output_witnesses_string.push(']'); - println!("{output_witnesses_string}"); + let witnesses = match info { + LogInfo::FinalizedOutput(output_string) => { + println!("{output_string}"); + return Ok(()); + } + LogInfo::WitnessOutput(witnesses) => witnesses, + }; + + if witnesses.len() == 1 { + match initial_witness.entry(witnesses[0]) { + std::collections::btree_map::Entry::Vacant(_) => { + unreachable!("log entry does must have a witness"); + } + std::collections::btree_map::Entry::Occupied(e) => { + println!("{}", e.get().to_hex()); } } + return Ok(()); } + + // If multiple witnesses are to be fetched for a log directive, + // it assumed that an array is meant to be printed to standard output + let mut output_witnesses_string = "".to_owned(); + output_witnesses_string.push('['); + let mut iter = witnesses.iter().peekable(); + while let Some(w) = iter.next() { + let elem = match initial_witness.entry(*w) { + std::collections::btree_map::Entry::Vacant(_) => { + return Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::MissingAssignment(w.0), + )) + } + std::collections::btree_map::Entry::Occupied(e) => *e.get(), + }; + if iter.peek().is_none() { + output_witnesses_string.push_str(&elem.to_hex()); + } else { + output_witnesses_string.push_str(&format!("{}, ", elem.to_hex())); + } + } + + output_witnesses_string.push(']'); + println!("{output_witnesses_string}"); + Ok(()) } } From efeca11307241c3916602b8f0fb98e1ca75203ea Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Fri, 3 Feb 2023 23:26:25 +0000 Subject: [PATCH 06/12] wrap comment --- acir/src/circuit/directives.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/acir/src/circuit/directives.rs b/acir/src/circuit/directives.rs index b7b91cdbc..cd94fde0e 100644 --- a/acir/src/circuit/directives.rs +++ b/acir/src/circuit/directives.rs @@ -200,8 +200,10 @@ 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 gen. -// Otherwise, we must store witnesses whose values will be fetched during the PWG. +// 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 { FinalizedOutput(String), WitnessOutput(Vec), From 2e3a64f864f49017131e068339ccc8923f25fea9 Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Fri, 3 Feb 2023 23:43:15 +0000 Subject: [PATCH 07/12] use get&expect instead of match --- acvm/src/pwg/directives.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 9644949bc..159873083 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -124,14 +124,14 @@ pub fn solve_directives( }; if witnesses.len() == 1 { - match initial_witness.entry(witnesses[0]) { - std::collections::btree_map::Entry::Vacant(_) => { - unreachable!("log entry does must have a witness"); - } - std::collections::btree_map::Entry::Occupied(e) => { - println!("{}", e.get().to_hex()); - } - } + let witness = &witnesses[0]; + + let e = initial_witness + .get(witness) + .expect("log entry does must have a witness"); + + println!("{}", e.to_hex()); + return Ok(()); } From 5fa1b90b1f09ba1cca515e171939c264387fb931 Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Fri, 3 Feb 2023 23:44:46 +0000 Subject: [PATCH 08/12] refactor error message --- acvm/src/pwg/directives.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 159873083..5dedb66b4 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -126,9 +126,9 @@ pub fn solve_directives( if witnesses.len() == 1 { let witness = &witnesses[0]; - let e = initial_witness - .get(witness) - .expect("log entry does must have a witness"); + let e = initial_witness.get(witness).expect( + "infallible: initial witness should contain the given witness index {witness}", + ); println!("{}", e.to_hex()); From 6589798d6716c76cee3d62ab7e15d356ea6cb2c0 Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Fri, 3 Feb 2023 23:47:40 +0000 Subject: [PATCH 09/12] use witness_to_value instead of panicking --- acvm/src/pwg/directives.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 5dedb66b4..83cce83ac 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -125,12 +125,8 @@ pub fn solve_directives( if witnesses.len() == 1 { let witness = &witnesses[0]; - - let e = initial_witness.get(witness).expect( - "infallible: initial witness should contain the given witness index {witness}", - ); - - println!("{}", e.to_hex()); + let log_value = witness_to_value(initial_witness, *witness)?; + println!("{}", log_value.to_hex()); return Ok(()); } From a8bcccd23b160cb0b696eb166526d49733967926 Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Fri, 3 Feb 2023 23:49:17 +0000 Subject: [PATCH 10/12] multi: - use witness_to_value - rename elem to element --- acvm/src/pwg/directives.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 83cce83ac..eb2d7571c 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -136,19 +136,13 @@ pub fn solve_directives( let mut output_witnesses_string = "".to_owned(); output_witnesses_string.push('['); let mut iter = witnesses.iter().peekable(); + while let Some(w) = iter.next() { - let elem = match initial_witness.entry(*w) { - std::collections::btree_map::Entry::Vacant(_) => { - return Err(OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::MissingAssignment(w.0), - )) - } - std::collections::btree_map::Entry::Occupied(e) => *e.get(), - }; + let element = witness_to_value(initial_witness, *w)?; if iter.peek().is_none() { - output_witnesses_string.push_str(&elem.to_hex()); + output_witnesses_string.push_str(&element.to_hex()); } else { - output_witnesses_string.push_str(&format!("{}, ", elem.to_hex())); + output_witnesses_string.push_str(&format!("{}, ", element.to_hex())); } } From 70357d57bac8d488c87f00637e51b471f5f19723 Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Sat, 4 Feb 2023 00:02:58 +0000 Subject: [PATCH 11/12] rewrite printing logic using `.join` --- acvm/src/pwg/directives.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index eb2d7571c..0934d06c7 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -133,20 +133,20 @@ pub fn solve_directives( // If multiple witnesses are to be fetched for a log directive, // it assumed that an array is meant to be printed to standard output - let mut output_witnesses_string = "".to_owned(); - output_witnesses_string.push('['); - let mut iter = witnesses.iter().peekable(); - - while let Some(w) = iter.next() { - let element = witness_to_value(initial_witness, *w)?; - if iter.peek().is_none() { - output_witnesses_string.push_str(&element.to_hex()); - } else { - output_witnesses_string.push_str(&format!("{}, ", element.to_hex())); - } + // + // Collect all field element values corresponding to the given witness indices + // and convert them to hex strings. + let mut elements_as_hex = Vec::with_capacity(witnesses.len()); + for witness in witnesses { + let element = witness_to_value(initial_witness, *witness)?; + elements_as_hex.push(element.to_hex()); } - output_witnesses_string.push(']'); + // Join all of the hex strings using a comma + let comma_separated_elements = elements_as_hex.join(","); + + let output_witnesses_string = "[".to_owned() + &comma_separated_elements + "]"; + println!("{output_witnesses_string}"); Ok(()) From 520c2c0dc35103859c1bf8735448b348de22bc1f Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Sat, 4 Feb 2023 00:03:14 +0000 Subject: [PATCH 12/12] remove unused import --- acvm/src/pwg/directives.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 0934d06c7..ebe01fb2d 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -8,7 +8,7 @@ use acir::{ use num_bigint::BigUint; use num_traits::{One, Zero}; -use crate::{OpcodeNotSolvable, OpcodeResolutionError}; +use crate::OpcodeResolutionError; use super::{get_value, witness_to_value};