From 2f68fe8f3dfa30f034a27eaf0050625ad8ebe7bd Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 30 Jan 2023 16:57:16 +0000 Subject: [PATCH 01/24] feat: track all parameter witnesses in evaluator --- crates/noirc_evaluator/src/lib.rs | 70 ++++++++++--------- .../src/ssa/acir_gen/operations/return.rs | 7 +- .../src/ssa/acir_gen/operations/sort.rs | 2 +- 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index a4b81469f2d..ee71e2f1021 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -10,7 +10,7 @@ use acvm::{ }; use errors::{RuntimeError, RuntimeErrorKind}; use iter_extended::btree_map; -use noirc_abi::{AbiType, AbiVisibility}; +use noirc_abi::AbiType; use noirc_frontend::monomorphization::ast::*; use ssa::{node, ssa_gen::IrGenerator}; use std::collections::BTreeMap; @@ -25,7 +25,7 @@ pub struct Evaluator { // This is the number of witnesses indices used when // creating the private/public inputs of the ABI. num_witnesses_abi_len: usize, - public_inputs: Vec, + param_witnesses: BTreeMap>, opcodes: Vec, } @@ -43,7 +43,17 @@ pub fn create_circuit( let mut evaluator = Evaluator::new(); // First evaluate the main function - evaluator.evaluate_main_alt(program, enable_logging)?; + evaluator.evaluate_main_alt(program.clone(), enable_logging)?; + + // TODO: shrink this ungodly mess. + let public_inputs = program + .abi + .public_abi() + .parameter_names() + .into_iter() + .cloned() + .flat_map(|param_name| evaluator.param_witnesses[¶m_name].clone()) + .collect(); let witness_index = evaluator.current_witness_index(); @@ -51,7 +61,7 @@ pub fn create_circuit( Circuit { current_witness_index: witness_index, opcodes: evaluator.opcodes, - public_inputs: PublicInputs(evaluator.public_inputs), + public_inputs: PublicInputs(public_inputs), }, np_language, is_blackbox_supported, @@ -64,8 +74,8 @@ pub fn create_circuit( impl Evaluator { fn new() -> Self { Evaluator { - public_inputs: Vec::new(), num_witnesses_abi_len: 0, + param_witnesses: BTreeMap::new(), // XXX: Barretenberg, reserves the first index to have value 0. // When we increment, we do not use this index at all. // This means that every constraint system at the moment, will either need @@ -90,7 +100,8 @@ impl Evaluator { // an intermediate variable. let is_intermediate_variable = witness_index.as_usize() > self.num_witnesses_abi_len; - let is_public_input = self.public_inputs.contains(&witness_index); + let is_public_input = + self.param_witnesses.values().flatten().any(|&index| index == witness_index); !is_intermediate_variable && !is_public_input } @@ -140,46 +151,40 @@ impl Evaluator { name: &str, def: Definition, param_type: &AbiType, - visibility: &AbiVisibility, ir_gen: &mut IrGenerator, ) -> Result<(), RuntimeErrorKind> { - match param_type { + let witnesses = match param_type { AbiType::Field => { let witness = self.add_witness_to_cs(); - if *visibility == AbiVisibility::Public { - self.public_inputs.push(witness); - } ir_gen.create_new_variable( name.to_owned(), Some(def), node::ObjectType::NativeField, Some(witness), ); + vec![witness] } AbiType::Array { length, typ } => { let witnesses = self.generate_array_witnesses(length, typ)?; - if *visibility == AbiVisibility::Public { - self.public_inputs.extend(witnesses.clone()); - } - ir_gen.abi_array(name, Some(def), typ.as_ref(), *length, witnesses); + + ir_gen.abi_array(name, Some(def), typ.as_ref(), *length, witnesses.clone()); + witnesses } AbiType::Integer { sign: _, width } => { let witness = self.add_witness_to_cs(); ssa::acir_gen::range_constraint(witness, *width, self)?; - if *visibility == AbiVisibility::Public { - self.public_inputs.push(witness); - } let obj_type = ir_gen.get_object_type_from_abi(param_type); // Fetch signedness of the integer ir_gen.create_new_variable(name.to_owned(), Some(def), obj_type, Some(witness)); + + vec![witness] } AbiType::Boolean => { let witness = self.add_witness_to_cs(); ssa::acir_gen::range_constraint(witness, 1, self)?; - if *visibility == AbiVisibility::Public { - self.public_inputs.push(witness); - } let obj_type = node::ObjectType::Boolean; ir_gen.create_new_variable(name.to_owned(), Some(def), obj_type, Some(witness)); + + vec![witness] } AbiType::Struct { fields } => { let new_fields = btree_map(fields, |(inner_name, value)| { @@ -189,22 +194,20 @@ impl Evaluator { let mut struct_witnesses: BTreeMap> = BTreeMap::new(); self.generate_struct_witnesses(&mut struct_witnesses, &new_fields)?; - if *visibility == AbiVisibility::Public { - let witnesses: Vec = - struct_witnesses.values().flatten().cloned().collect(); - self.public_inputs.extend(witnesses); - } - ir_gen.abi_struct(name, Some(def), fields, struct_witnesses); + + ir_gen.abi_struct(name, Some(def), fields, struct_witnesses.clone()); + struct_witnesses.values().flatten().cloned().collect() } AbiType::String { length } => { let typ = AbiType::Integer { sign: noirc_abi::Sign::Unsigned, width: 8 }; let witnesses = self.generate_array_witnesses(length, &typ)?; - if *visibility == AbiVisibility::Public { - self.public_inputs.extend(witnesses.clone()); - } - ir_gen.abi_array(name, Some(def), &typ, *length, witnesses); + ir_gen.abi_array(name, Some(def), &typ, *length, witnesses.clone()); + witnesses } - } + }; + + self.param_witnesses.insert(name.to_owned(), witnesses); + Ok(()) } @@ -298,8 +301,7 @@ impl Evaluator { for ((param_id, _, param_name, _), abi_param) in main_params.iter().zip(abi_params) { assert_eq!(param_name, &abi_param.name); let def = Definition::Local(*param_id); - self.param_to_var(param_name, def, &abi_param.typ, &abi_param.visibility, ir_gen) - .unwrap(); + self.param_to_var(param_name, def, &abi_param.typ, ir_gen).unwrap(); } // Store the number of witnesses used to represent the types diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs index 96b637d99c2..7e20dad11e5 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs @@ -1,3 +1,6 @@ +use acvm::acir::native_types::Witness; +use noirc_abi::MAIN_RETURN_NAME; + use crate::{ errors::RuntimeErrorKind, ssa::{ @@ -37,6 +40,7 @@ pub(crate) fn evaluate( } }; + let mut witnesses: Vec = Vec::new(); for mut object in objects { let witness = object.get_or_compute_witness(evaluator, true).expect( "infallible: `None` can only be returned when we disallow constant Expressions.", @@ -48,8 +52,9 @@ pub(crate) fn evaluate( "we do not allow private ABI inputs to be returned as public outputs", ))); } - evaluator.public_inputs.push(witness); + witnesses.push(witness); } + evaluator.param_witnesses.insert(MAIN_RETURN_NAME.to_owned(), witnesses); } Ok(None) diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/sort.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/sort.rs index 430d1180b6e..98b72294229 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/sort.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/sort.rs @@ -122,7 +122,7 @@ mod test { let mut eval = Evaluator { current_witness_index: 0, num_witnesses_abi_len: 0, - public_inputs: Vec::new(), + param_witnesses: BTreeMap::new(), opcodes: Vec::new(), }; From 73a083e52f2a9d24429ca1bd8ac4cb301ca797cb Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 30 Jan 2023 18:09:16 +0000 Subject: [PATCH 02/24] chore: use explicit witness map for encoding/decoding witness map --- crates/nargo/src/cli/execute_cmd.rs | 89 +++++++++++-------- crates/noirc_abi/src/lib.rs | 127 ++++++++++++++-------------- crates/noirc_driver/src/lib.rs | 30 ++++--- crates/noirc_evaluator/src/lib.rs | 4 +- 4 files changed, 135 insertions(+), 115 deletions(-) diff --git a/crates/nargo/src/cli/execute_cmd.rs b/crates/nargo/src/cli/execute_cmd.rs index a89a0fcaa21..d88098c03c4 100644 --- a/crates/nargo/src/cli/execute_cmd.rs +++ b/crates/nargo/src/cli/execute_cmd.rs @@ -1,11 +1,13 @@ -use clap::ArgMatches; +use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use acvm::acir::native_types::Witness; use acvm::{FieldElement, PartialWitnessGenerator}; +use clap::ArgMatches; +use iter_extended::{try_btree_map, vecmap}; use noirc_abi::errors::AbiError; use noirc_abi::input_parser::{Format, InputValue}; -use noirc_abi::{Abi, MAIN_RETURN_NAME}; +use noirc_abi::{decode_value, encode_value, AbiParameter, MAIN_RETURN_NAME}; use noirc_driver::CompiledProgram; use super::{create_named_dir, read_inputs_from_file, write_to_file, InputMap, WitnessMap}; @@ -40,10 +42,6 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> { Ok(()) } -/// In Barretenberg, the proof system adds a zero witness in the first index, -/// So when we add witness values, their index start from 1. -const WITNESS_OFFSET: u32 = 1; - fn execute_with_path>( program_dir: P, show_ssa: bool, @@ -75,30 +73,12 @@ pub(crate) fn execute_program( Ok((return_value, solved_witness)) } -pub(crate) fn extract_public_inputs( - compiled_program: &CompiledProgram, - solved_witness: &WitnessMap, -) -> Result { - let encoded_public_inputs: Vec = compiled_program - .circuit - .public_inputs - .0 - .iter() - .map(|index| solved_witness[index]) - .collect(); - - let public_abi = compiled_program.abi.as_ref().unwrap().clone().public_abi(); - - public_abi.decode(&encoded_public_inputs) -} - pub(crate) fn solve_witness( compiled_program: &CompiledProgram, input_map: &InputMap, ) -> Result { - let abi = compiled_program.abi.as_ref().unwrap().clone(); - let mut solved_witness = - input_map_to_witness_map(abi, input_map).map_err(|error| match error { + let mut solved_witness = input_map_to_witness_map(input_map, &compiled_program.param_witnesses) + .map_err(|error| match error { AbiError::UndefinedInput(_) => { CliError::Generic(format!("{error} in the {PROVER_INPUT_FILE}.toml file.")) } @@ -115,18 +95,53 @@ pub(crate) fn solve_witness( /// /// In particular, this method shows one how to associate values in a Toml/JSON /// file with witness indices -fn input_map_to_witness_map(abi: Abi, input_map: &InputMap) -> Result { - // The ABI map is first encoded as a vector of field elements - let encoded_inputs = abi.encode(input_map, true)?; - - Ok(encoded_inputs - .into_iter() - .enumerate() - .map(|(index, witness_value)| { - let witness = Witness::new(WITNESS_OFFSET + (index as u32)); - (witness, witness_value) +fn input_map_to_witness_map( + input_map: &InputMap, + abi_witness_map: &BTreeMap>, +) -> Result { + // First encode each input separately + let encoded_input_map: BTreeMap> = + try_btree_map(input_map, |(key, value)| { + encode_value(value.clone(), key).map(|v| (key.clone(), v)) + })?; + + // Write input field elements into witness indices specified in `abi_witness_map`. + let witness_map = encoded_input_map + .iter() + .flat_map(|(param_name, encoded_param_fields)| { + let param_witness_indices = &abi_witness_map[param_name]; + param_witness_indices + .iter() + .zip(encoded_param_fields.iter()) + .map(|(&witness, &field_element)| (witness, field_element)) + }) + .collect(); + + Ok(witness_map) +} + +pub(crate) fn extract_public_inputs( + compiled_program: &CompiledProgram, + solved_witness: &WitnessMap, +) -> Result { + let public_abi = compiled_program.abi.as_ref().unwrap().clone().public_abi(); + + let public_inputs_map = public_abi + .parameters + .iter() + .map(|AbiParameter { name, typ, .. }| { + let param_witness_values = + vecmap(compiled_program.param_witnesses[name].clone(), |witness_index| { + solved_witness[&witness_index] + }); + + decode_value(&mut param_witness_values.into_iter(), typ) + .map(|input_value| (name.clone(), input_value)) + .unwrap() }) - .collect()) + .collect(); + + Ok(public_inputs_map) } pub(crate) fn save_witness_to_dir>( diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 7d8bc645102..ad0606174f5 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -183,7 +183,7 @@ impl Abi { return Err(AbiError::TypeMismatch { param: param.to_owned(), value }); } - encoded_inputs.extend(Self::encode_value(value, ¶m.name)?); + encoded_inputs.extend(encode_value(value, ¶m.name)?); } // Check that no extra witness values have been provided. @@ -197,27 +197,6 @@ impl Abi { Ok(encoded_inputs) } - fn encode_value(value: InputValue, param_name: &String) -> Result, AbiError> { - let mut encoded_value = Vec::new(); - match value { - InputValue::Field(elem) => encoded_value.push(elem), - InputValue::Vec(vec_elem) => encoded_value.extend(vec_elem), - InputValue::String(string) => { - let str_as_fields = - string.bytes().map(|byte| FieldElement::from_be_bytes_reduce(&[byte])); - encoded_value.extend(str_as_fields) - } - InputValue::Struct(object) => { - for (field_name, value) in object { - let new_name = format!("{param_name}.{field_name}"); - encoded_value.extend(Self::encode_value(value, &new_name)?) - } - } - InputValue::Undefined => return Err(AbiError::UndefinedInput(param_name.to_string())), - } - Ok(encoded_value) - } - /// Decode a vector of `FieldElements` into the types specified in the ABI. pub fn decode( &self, @@ -234,59 +213,79 @@ impl Abi { let mut field_iterator = encoded_inputs.iter().cloned(); let mut decoded_inputs = BTreeMap::new(); for param in &self.parameters { - let decoded_value = Self::decode_value(&mut field_iterator, ¶m.typ)?; + let decoded_value = decode_value(&mut field_iterator, ¶m.typ)?; decoded_inputs.insert(param.name.to_owned(), decoded_value); } Ok(decoded_inputs) } +} - fn decode_value( - field_iterator: &mut impl Iterator, - value_type: &AbiType, - ) -> Result { - // This function assumes that `field_iterator` contains enough `FieldElement`s in order to decode a `value_type` - // `Abi.decode` enforces that the encoded inputs matches the expected length defined by the ABI so this is safe. - let value = match value_type { - AbiType::Field | AbiType::Integer { .. } | AbiType::Boolean => { - let field_element = field_iterator.next().unwrap(); - - InputValue::Field(field_element) +pub fn encode_value(value: InputValue, param_name: &String) -> Result, AbiError> { + let mut encoded_value = Vec::new(); + match value { + InputValue::Field(elem) => encoded_value.push(elem), + InputValue::Vec(vec_elem) => encoded_value.extend(vec_elem), + InputValue::String(string) => { + let str_as_fields = + string.bytes().map(|byte| FieldElement::from_be_bytes_reduce(&[byte])); + encoded_value.extend(str_as_fields) + } + InputValue::Struct(object) => { + for (field_name, value) in object { + let new_name = format!("{param_name}.{field_name}"); + encoded_value.extend(encode_value(value, &new_name)?) } - AbiType::Array { length, .. } => { - let field_elements: Vec = - field_iterator.take(*length as usize).collect(); + } + InputValue::Undefined => return Err(AbiError::UndefinedInput(param_name.to_string())), + } + Ok(encoded_value) +} - InputValue::Vec(field_elements) - } - AbiType::String { length } => { - let string_as_slice: Vec = field_iterator - .take(*length as usize) - .map(|e| { - let mut field_as_bytes = e.to_be_bytes(); - let char_byte = field_as_bytes.pop().unwrap(); // A character in a string is represented by a u8, thus we just want the last byte of the element - assert!(field_as_bytes.into_iter().all(|b| b == 0)); // Assert that the rest of the field element's bytes are empty - char_byte - }) - .collect(); - - let final_string = str::from_utf8(&string_as_slice).unwrap(); - - InputValue::String(final_string.to_owned()) - } - AbiType::Struct { fields, .. } => { - let mut struct_map = BTreeMap::new(); +pub fn decode_value( + field_iterator: &mut impl Iterator, + value_type: &AbiType, +) -> Result { + // This function assumes that `field_iterator` contains enough `FieldElement`s in order to decode a `value_type` + // `Abi.decode` enforces that the encoded inputs matches the expected length defined by the ABI so this is safe. + let value = match value_type { + AbiType::Field | AbiType::Integer { .. } | AbiType::Boolean => { + let field_element = field_iterator.next().unwrap(); + + InputValue::Field(field_element) + } + AbiType::Array { length, .. } => { + let field_elements: Vec = field_iterator.take(*length as usize).collect(); - for (field_key, param_type) in fields { - let field_value = Self::decode_value(field_iterator, param_type)?; + InputValue::Vec(field_elements) + } + AbiType::String { length } => { + let string_as_slice: Vec = field_iterator + .take(*length as usize) + .map(|e| { + let mut field_as_bytes = e.to_be_bytes(); + let char_byte = field_as_bytes.pop().unwrap(); // A character in a string is represented by a u8, thus we just want the last byte of the element + assert!(field_as_bytes.into_iter().all(|b| b == 0)); // Assert that the rest of the field element's bytes are empty + char_byte + }) + .collect(); + + let final_string = str::from_utf8(&string_as_slice).unwrap(); + + InputValue::String(final_string.to_owned()) + } + AbiType::Struct { fields, .. } => { + let mut struct_map = BTreeMap::new(); - struct_map.insert(field_key.to_owned(), field_value); - } + for (field_key, param_type) in fields { + let field_value = decode_value(field_iterator, param_type)?; - InputValue::Struct(struct_map) + struct_map.insert(field_key.to_owned(), field_value); } - }; - Ok(value) - } + InputValue::Struct(struct_map) + } + }; + + Ok(value) } diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 69d19517585..116161f9a62 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -1,6 +1,7 @@ #![forbid(unsafe_code)] use acvm::acir::circuit::Circuit; +use acvm::acir::native_types::Witness; use acvm::Language; use fm::FileType; use noirc_abi::Abi; @@ -12,6 +13,7 @@ use noirc_frontend::hir::Context; use noirc_frontend::monomorphization::monomorphize; use noirc_frontend::node_interner::FuncId; use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; use std::path::{Path, PathBuf}; pub struct Driver { @@ -22,6 +24,7 @@ pub struct Driver { pub struct CompiledProgram { pub circuit: Circuit, pub abi: Option, + pub param_witnesses: BTreeMap>, } impl Driver { @@ -187,18 +190,21 @@ impl Driver { let program = monomorphize(main_function, &self.context.def_interner); let blackbox_supported = acvm::default_is_black_box_supported(np_language.clone()); - match create_circuit(program, np_language, blackbox_supported, show_ssa) { - Ok(circuit) => Ok(CompiledProgram { circuit, abi: Some(abi) }), - Err(err) => { - // The FileId here will be the file id of the file with the main file - // Errors will be shown at the call site without a stacktrace - let file = err.location.map(|loc| loc.file); - let files = &self.context.file_manager; - let error = reporter::report(files, &err.into(), file, allow_warnings); - reporter::finish_report(error as u32)?; - Err(ReportedError) - } - } + let (circuit, param_witnesses) = + match create_circuit(program, np_language, blackbox_supported, show_ssa) { + Ok((circuit, param_witnesses)) => (circuit, param_witnesses), + Err(err) => { + // The FileId here will be the file id of the file with the main file + // Errors will be shown at the call site without a stacktrace + let file = err.location.map(|loc| loc.file); + let files = &self.context.file_manager; + let error = reporter::report(files, &err.into(), file, allow_warnings); + reporter::finish_report(error as u32)?; + return Err(ReportedError); + } + }; + + Ok(CompiledProgram { circuit, abi: Some(abi), param_witnesses }) } /// Returns a list of all functions in the current crate marked with #[test] diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index ee71e2f1021..a05b94d001c 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -39,7 +39,7 @@ pub fn create_circuit( np_language: Language, is_blackbox_supported: IsBlackBoxSupported, enable_logging: bool, -) -> Result { +) -> Result<(Circuit, BTreeMap>), RuntimeError> { let mut evaluator = Evaluator::new(); // First evaluate the main function @@ -68,7 +68,7 @@ pub fn create_circuit( ) .map_err(|_| RuntimeErrorKind::Spanless(String::from("produced an acvm compile error")))?; - Ok(optimized_circuit) + Ok((optimized_circuit, evaluator.param_witnesses)) } impl Evaluator { From 123f09d2f3d09dec1f53c3e0cc77b66b189dedfb Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 15 Feb 2023 11:36:27 +0000 Subject: [PATCH 03/24] chore: move `param_witnesses` to be stored within ABI --- crates/nargo/src/cli/execute_cmd.rs | 24 +++++++++---------- crates/nargo/src/cli/prove_cmd.rs | 3 ++- crates/noirc_abi/src/lib.rs | 10 ++++++-- crates/noirc_driver/src/lib.rs | 10 ++++---- crates/noirc_frontend/src/hir_def/function.rs | 4 +++- 5 files changed, 29 insertions(+), 22 deletions(-) diff --git a/crates/nargo/src/cli/execute_cmd.rs b/crates/nargo/src/cli/execute_cmd.rs index d88098c03c4..cd38e98ace6 100644 --- a/crates/nargo/src/cli/execute_cmd.rs +++ b/crates/nargo/src/cli/execute_cmd.rs @@ -7,7 +7,7 @@ use clap::ArgMatches; use iter_extended::{try_btree_map, vecmap}; use noirc_abi::errors::AbiError; use noirc_abi::input_parser::{Format, InputValue}; -use noirc_abi::{decode_value, encode_value, AbiParameter, MAIN_RETURN_NAME}; +use noirc_abi::{decode_value, encode_value, Abi, AbiParameter, MAIN_RETURN_NAME}; use noirc_driver::CompiledProgram; use super::{create_named_dir, read_inputs_from_file, write_to_file, InputMap, WitnessMap}; @@ -67,7 +67,8 @@ pub(crate) fn execute_program( // Solve the remaining witnesses let solved_witness = solve_witness(compiled_program, inputs_map)?; - let public_inputs = extract_public_inputs(compiled_program, &solved_witness)?; + let public_abi = compiled_program.abi.as_ref().unwrap().clone().public_abi(); + let public_inputs = extract_public_inputs(&public_abi, &solved_witness)?; let return_value = public_inputs.get(MAIN_RETURN_NAME).cloned(); Ok((return_value, solved_witness)) @@ -77,8 +78,10 @@ pub(crate) fn solve_witness( compiled_program: &CompiledProgram, input_map: &InputMap, ) -> Result { - let mut solved_witness = input_map_to_witness_map(input_map, &compiled_program.param_witnesses) - .map_err(|error| match error { + let abi = compiled_program.abi.as_ref().unwrap(); + + let mut solved_witness = + input_map_to_witness_map(abi, input_map).map_err(|error| match error { AbiError::UndefinedInput(_) => { CliError::Generic(format!("{error} in the {PROVER_INPUT_FILE}.toml file.")) } @@ -95,10 +98,7 @@ pub(crate) fn solve_witness( /// /// In particular, this method shows one how to associate values in a Toml/JSON /// file with witness indices -fn input_map_to_witness_map( - input_map: &InputMap, - abi_witness_map: &BTreeMap>, -) -> Result { +fn input_map_to_witness_map(abi: &Abi, input_map: &InputMap) -> Result { // First encode each input separately let encoded_input_map: BTreeMap> = try_btree_map(input_map, |(key, value)| { @@ -109,7 +109,7 @@ fn input_map_to_witness_map( let witness_map = encoded_input_map .iter() .flat_map(|(param_name, encoded_param_fields)| { - let param_witness_indices = &abi_witness_map[param_name]; + let param_witness_indices = &abi.param_witnesses[param_name]; param_witness_indices .iter() .zip(encoded_param_fields.iter()) @@ -121,17 +121,15 @@ fn input_map_to_witness_map( } pub(crate) fn extract_public_inputs( - compiled_program: &CompiledProgram, + public_abi: &Abi, solved_witness: &WitnessMap, ) -> Result { - let public_abi = compiled_program.abi.as_ref().unwrap().clone().public_abi(); - let public_inputs_map = public_abi .parameters .iter() .map(|AbiParameter { name, typ, .. }| { let param_witness_values = - vecmap(compiled_program.param_witnesses[name].clone(), |witness_index| { + vecmap(public_abi.param_witnesses[name].clone(), |witness_index| { solved_witness[&witness_index] }); diff --git a/crates/nargo/src/cli/prove_cmd.rs b/crates/nargo/src/cli/prove_cmd.rs index d67cdceffb9..e0009b6ea9b 100644 --- a/crates/nargo/src/cli/prove_cmd.rs +++ b/crates/nargo/src/cli/prove_cmd.rs @@ -57,7 +57,8 @@ pub fn prove_with_path>( let (_, solved_witness) = execute_program(&compiled_program, &inputs_map)?; // Write public inputs into Verifier.toml - let public_inputs = extract_public_inputs(&compiled_program, &solved_witness)?; + let public_abi = compiled_program.abi.as_ref().unwrap().clone().public_abi(); + let public_inputs = extract_public_inputs(&public_abi, &solved_witness)?; write_inputs_to_file(&public_inputs, &program_dir, VERIFIER_INPUT_FILE, Format::Toml)?; // Since the public outputs are added onto the public inputs list, there can be duplicates. diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index ad0606174f5..c5e80b6c1f6 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -1,7 +1,7 @@ #![forbid(unsafe_code)] use std::{collections::BTreeMap, convert::TryInto, str}; -use acvm::FieldElement; +use acvm::{acir::native_types::Witness, FieldElement}; use errors::AbiError; use input_parser::InputValue; use serde::{Deserialize, Serialize}; @@ -120,6 +120,7 @@ impl AbiParameter { #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Abi { pub parameters: Vec, + pub param_witnesses: BTreeMap>, } impl Abi { @@ -149,7 +150,12 @@ impl Abi { pub fn public_abi(self) -> Abi { let parameters: Vec<_> = self.parameters.into_iter().filter(|param| param.is_public()).collect(); - Abi { parameters } + let param_witnesses = self + .param_witnesses + .into_iter() + .filter(|(param_name, _)| parameters.iter().any(|param| ¶m.name == param_name)) + .collect(); + Abi { parameters, param_witnesses } } /// Encode a set of inputs as described in the ABI into a vector of `FieldElement`s. diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 116161f9a62..17e728c75fa 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -1,7 +1,6 @@ #![forbid(unsafe_code)] use acvm::acir::circuit::Circuit; -use acvm::acir::native_types::Witness; use acvm::Language; use fm::FileType; use noirc_abi::Abi; @@ -13,7 +12,6 @@ use noirc_frontend::hir::Context; use noirc_frontend::monomorphization::monomorphize; use noirc_frontend::node_interner::FuncId; use serde::{Deserialize, Serialize}; -use std::collections::BTreeMap; use std::path::{Path, PathBuf}; pub struct Driver { @@ -24,7 +22,6 @@ pub struct Driver { pub struct CompiledProgram { pub circuit: Circuit, pub abi: Option, - pub param_witnesses: BTreeMap>, } impl Driver { @@ -185,7 +182,7 @@ impl Driver { // Create ABI for main function let func_meta = self.context.def_interner.function_meta(&main_function); - let abi = func_meta.into_abi(&self.context.def_interner); + let mut abi = func_meta.into_abi(&self.context.def_interner); let program = monomorphize(main_function, &self.context.def_interner); @@ -204,7 +201,10 @@ impl Driver { } }; - Ok(CompiledProgram { circuit, abi: Some(abi), param_witnesses }) + // TODO: Try to avoid this hack. + abi.param_witnesses = param_witnesses; + + Ok(CompiledProgram { circuit, abi: Some(abi) }) } /// Returns a list of all functions in the current crate marked with #[test] diff --git a/crates/noirc_frontend/src/hir_def/function.rs b/crates/noirc_frontend/src/hir_def/function.rs index 2fa9d5edf1b..4f295fee90a 100644 --- a/crates/noirc_frontend/src/hir_def/function.rs +++ b/crates/noirc_frontend/src/hir_def/function.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeMap; + use iter_extended::vecmap; use noirc_abi::{Abi, AbiParameter, AbiVisibility, MAIN_RETURN_NAME}; use noirc_errors::{Location, Span}; @@ -61,7 +63,7 @@ impl Parameters { let as_abi = param.1.as_abi_type(); AbiParameter { name: param_name, typ: as_abi, visibility: param.2 } }); - noirc_abi::Abi { parameters } + noirc_abi::Abi { parameters, param_witnesses: BTreeMap::new() } } pub fn span(&self) -> Span { From 0f829edf8fe8cfd709ef1176dd55d514798deb40 Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 3 Feb 2023 18:03:37 +0000 Subject: [PATCH 04/24] feat: create separate methods for encoding to a witness vs verifier inputs --- crates/nargo/src/cli/execute_cmd.rs | 71 ++++------------------- crates/nargo/src/cli/mod.rs | 18 +----- crates/nargo/src/cli/prove_cmd.rs | 7 +-- crates/nargo/src/cli/verify_cmd.rs | 2 +- crates/noirc_abi/src/lib.rs | 90 ++++++++++++++++++++++------- 5 files changed, 86 insertions(+), 102 deletions(-) diff --git a/crates/nargo/src/cli/execute_cmd.rs b/crates/nargo/src/cli/execute_cmd.rs index cd38e98ace6..a9b6600b60e 100644 --- a/crates/nargo/src/cli/execute_cmd.rs +++ b/crates/nargo/src/cli/execute_cmd.rs @@ -1,16 +1,14 @@ -use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use acvm::acir::native_types::Witness; -use acvm::{FieldElement, PartialWitnessGenerator}; +use acvm::PartialWitnessGenerator; use clap::ArgMatches; -use iter_extended::{try_btree_map, vecmap}; use noirc_abi::errors::AbiError; use noirc_abi::input_parser::{Format, InputValue}; -use noirc_abi::{decode_value, encode_value, Abi, AbiParameter, MAIN_RETURN_NAME}; +use noirc_abi::{InputMap, WitnessMap, MAIN_RETURN_NAME}; use noirc_driver::CompiledProgram; -use super::{create_named_dir, read_inputs_from_file, write_to_file, InputMap, WitnessMap}; +use super::{create_named_dir, read_inputs_from_file, write_to_file}; use crate::{ cli::compile_cmd::compile_circuit, constants::{PROVER_INPUT_FILE, TARGET_DIR, WITNESS_EXT}, @@ -68,7 +66,7 @@ pub(crate) fn execute_program( let solved_witness = solve_witness(compiled_program, inputs_map)?; let public_abi = compiled_program.abi.as_ref().unwrap().clone().public_abi(); - let public_inputs = extract_public_inputs(&public_abi, &solved_witness)?; + let public_inputs = public_abi.decode_from_witness(&solved_witness)?; let return_value = public_inputs.get(MAIN_RETURN_NAME).cloned(); Ok((return_value, solved_witness)) @@ -80,13 +78,12 @@ pub(crate) fn solve_witness( ) -> Result { let abi = compiled_program.abi.as_ref().unwrap(); - let mut solved_witness = - input_map_to_witness_map(abi, input_map).map_err(|error| match error { - AbiError::UndefinedInput(_) => { - CliError::Generic(format!("{error} in the {PROVER_INPUT_FILE}.toml file.")) - } - _ => CliError::from(error), - })?; + let mut solved_witness = abi.encode_to_witness(input_map).map_err(|error| match error { + AbiError::UndefinedInput(_) => { + CliError::Generic(format!("{error} in the {PROVER_INPUT_FILE}.toml file.")) + } + _ => CliError::from(error), + })?; let backend = crate::backends::ConcreteBackend; backend.solve(&mut solved_witness, compiled_program.circuit.opcodes.clone())?; @@ -94,54 +91,6 @@ pub(crate) fn solve_witness( Ok(solved_witness) } -/// Given an InputMap and an Abi, produce a WitnessMap -/// -/// In particular, this method shows one how to associate values in a Toml/JSON -/// file with witness indices -fn input_map_to_witness_map(abi: &Abi, input_map: &InputMap) -> Result { - // First encode each input separately - let encoded_input_map: BTreeMap> = - try_btree_map(input_map, |(key, value)| { - encode_value(value.clone(), key).map(|v| (key.clone(), v)) - })?; - - // Write input field elements into witness indices specified in `abi_witness_map`. - let witness_map = encoded_input_map - .iter() - .flat_map(|(param_name, encoded_param_fields)| { - let param_witness_indices = &abi.param_witnesses[param_name]; - param_witness_indices - .iter() - .zip(encoded_param_fields.iter()) - .map(|(&witness, &field_element)| (witness, field_element)) - }) - .collect(); - - Ok(witness_map) -} - -pub(crate) fn extract_public_inputs( - public_abi: &Abi, - solved_witness: &WitnessMap, -) -> Result { - let public_inputs_map = public_abi - .parameters - .iter() - .map(|AbiParameter { name, typ, .. }| { - let param_witness_values = - vecmap(public_abi.param_witnesses[name].clone(), |witness_index| { - solved_witness[&witness_index] - }); - - decode_value(&mut param_witness_values.into_iter(), typ) - .map(|input_value| (name.clone(), input_value)) - .unwrap() - }) - .collect(); - - Ok(public_inputs_map) -} - pub(crate) fn save_witness_to_dir>( witness: WitnessMap, witness_name: &str, diff --git a/crates/nargo/src/cli/mod.rs b/crates/nargo/src/cli/mod.rs index 92cc68b41d9..f9d8bbc42d3 100644 --- a/crates/nargo/src/cli/mod.rs +++ b/crates/nargo/src/cli/mod.rs @@ -1,19 +1,13 @@ -use acvm::{ - acir::{circuit::PublicInputs, native_types::Witness}, - FieldElement, -}; +use acvm::{acir::circuit::PublicInputs, FieldElement}; pub use check_cmd::check_from_path; use clap::{App, AppSettings, Arg}; use const_format::formatcp; use git_version::git_version; -use noirc_abi::{ - input_parser::{Format, InputValue}, - Abi, -}; +use noirc_abi::{input_parser::Format, Abi, InputMap}; use noirc_driver::Driver; use noirc_frontend::graph::{CrateName, CrateType}; use std::{ - collections::{BTreeMap, HashMap, HashSet}, + collections::{HashMap, HashSet}, fs::File, io::Write, path::{Path, PathBuf}, @@ -36,12 +30,6 @@ mod verify_cmd; const SHORT_GIT_HASH: &str = git_version!(prefix = "git:"); const VERSION_STRING: &str = formatcp!("{} ({})", env!("CARGO_PKG_VERSION"), SHORT_GIT_HASH); -/// A map from the fields in an TOML/JSON file which correspond to some ABI to their values -pub type InputMap = BTreeMap; - -/// A map from the witnesses in a constraint system to the field element values -pub type WitnessMap = BTreeMap; - pub fn start_cli() { let allow_warnings = Arg::with_name("allow-warnings") .long("allow-warnings") diff --git a/crates/nargo/src/cli/prove_cmd.rs b/crates/nargo/src/cli/prove_cmd.rs index e0009b6ea9b..9d2e78f7387 100644 --- a/crates/nargo/src/cli/prove_cmd.rs +++ b/crates/nargo/src/cli/prove_cmd.rs @@ -9,10 +9,7 @@ use super::{ write_to_file, }; use crate::{ - cli::{ - execute_cmd::{execute_program, extract_public_inputs}, - verify_cmd::verify_proof, - }, + cli::{execute_cmd::execute_program, verify_cmd::verify_proof}, constants::{PROOFS_DIR, PROOF_EXT, PROVER_INPUT_FILE, VERIFIER_INPUT_FILE}, errors::CliError, }; @@ -58,7 +55,7 @@ pub fn prove_with_path>( // Write public inputs into Verifier.toml let public_abi = compiled_program.abi.as_ref().unwrap().clone().public_abi(); - let public_inputs = extract_public_inputs(&public_abi, &solved_witness)?; + let public_inputs = public_abi.decode_from_witness(&solved_witness)?; write_inputs_to_file(&public_inputs, &program_dir, VERIFIER_INPUT_FILE, Format::Toml)?; // Since the public outputs are added onto the public inputs list, there can be duplicates. diff --git a/crates/nargo/src/cli/verify_cmd.rs b/crates/nargo/src/cli/verify_cmd.rs index 60efc882ec7..6f541c33482 100644 --- a/crates/nargo/src/cli/verify_cmd.rs +++ b/crates/nargo/src/cli/verify_cmd.rs @@ -64,7 +64,7 @@ pub(crate) fn verify_proof( ) -> Result { let public_abi = compiled_program.abi.unwrap().public_abi(); let public_inputs = - public_abi.encode(&public_inputs_map, false).map_err(|error| match error { + public_abi.encode_to_array(&public_inputs_map).map_err(|error| match error { AbiError::UndefinedInput(_) => { CliError::Generic(format!("{error} in the {VERIFIER_INPUT_FILE}.toml file.")) } diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index c5e80b6c1f6..197c40088e4 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -4,6 +4,7 @@ use std::{collections::BTreeMap, convert::TryInto, str}; use acvm::{acir::native_types::Witness, FieldElement}; use errors::AbiError; use input_parser::InputValue; +use iter_extended::{try_btree_map, vecmap}; use serde::{Deserialize, Serialize}; // This is the ABI used to bridge the different TOML formats for the initial @@ -15,6 +16,12 @@ pub mod errors; pub mod input_parser; mod serialization; +/// A map from the fields in an TOML/JSON file which correspond to some ABI to their values +pub type InputMap = BTreeMap; + +/// A map from the witnesses in a constraint system to the field element values +pub type WitnessMap = BTreeMap; + pub const MAIN_RETURN_NAME: &str = "return"; #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] @@ -158,35 +165,55 @@ impl Abi { Abi { parameters, param_witnesses } } + /// Encode a set of inputs as described in the ABI into a `WitnessMap`. + pub fn encode_to_witness( + &self, + input_map: &BTreeMap, + ) -> Result, AbiError> { + // TODO: Avoid this clone + let mut input_map = input_map.clone(); + + // Remove the return value parameter + input_map.remove(MAIN_RETURN_NAME); + + // First encode each input separately + let encoded_input_map: BTreeMap> = + try_btree_map(input_map, |(key, value)| { + encode_value(value, &key).map(|v| (key.clone(), v)) + })?; + + // Write input field elements into witness indices specified in `abi_witness_map`. + let witness_map = encoded_input_map + .iter() + .flat_map(|(param_name, encoded_param_fields)| { + let param_witness_indices = &self.param_witnesses[param_name]; + param_witness_indices + .iter() + .zip(encoded_param_fields.iter()) + .map(|(&witness, &field_element)| (witness, field_element)) + }) + .collect(); + + Ok(witness_map) + } + /// Encode a set of inputs as described in the ABI into a vector of `FieldElement`s. - pub fn encode( + pub fn encode_to_array( self, inputs: &BTreeMap, - skip_output: bool, ) -> Result, AbiError> { - // Condition that specifies whether we should filter the "return" - // parameter. We do this in the case that it is not in the `inputs` - // map specified. - // - // See Issue #645 : Adding a `public outputs` field into acir and - // the ABI will clean up this logic - // For prosperity; the prover does not know about a `return` value - // so we skip this when encoding the ABI - let return_condition = - |param_name: &&String| !skip_output || (param_name != &MAIN_RETURN_NAME); - - let parameters = self.parameters.iter().filter(|param| return_condition(&¶m.name)); - let param_names: Vec<&String> = parameters.clone().map(|param| ¶m.name).collect(); + let parameters = self.parameters.clone(); + let param_names: Vec<&String> = parameters.iter().map(|param| ¶m.name).collect(); let mut encoded_inputs = Vec::new(); - for param in parameters { + for param in self.parameters { let value = inputs .get(¶m.name) .ok_or_else(|| AbiError::MissingParam(param.name.to_owned()))? .clone(); if !value.matches_abi(¶m.typ) { - return Err(AbiError::TypeMismatch { param: param.to_owned(), value }); + return Err(AbiError::TypeMismatch { param, value }); } encoded_inputs.extend(encode_value(value, ¶m.name)?); @@ -203,8 +230,31 @@ impl Abi { Ok(encoded_inputs) } + /// Decode a `WitnessMap` into the types specified in the ABI. + pub fn decode_from_witness( + &self, + witness_map: &BTreeMap, + ) -> Result, AbiError> { + let public_inputs_map = self + .parameters + .iter() + .map(|AbiParameter { name, typ, .. }| { + let param_witness_values = + vecmap(self.param_witnesses[name].clone(), |witness_index| { + witness_map[&witness_index] + }); + + decode_value(&mut param_witness_values.into_iter(), typ) + .map(|input_value| (name.clone(), input_value)) + .unwrap() + }) + .collect(); + + Ok(public_inputs_map) + } + /// Decode a vector of `FieldElements` into the types specified in the ABI. - pub fn decode( + pub fn decode_from_array( &self, encoded_inputs: &[FieldElement], ) -> Result, AbiError> { @@ -227,7 +277,7 @@ impl Abi { } } -pub fn encode_value(value: InputValue, param_name: &String) -> Result, AbiError> { +fn encode_value(value: InputValue, param_name: &String) -> Result, AbiError> { let mut encoded_value = Vec::new(); match value { InputValue::Field(elem) => encoded_value.push(elem), @@ -248,7 +298,7 @@ pub fn encode_value(value: InputValue, param_name: &String) -> Result, value_type: &AbiType, ) -> Result { From f3a78f6eaa589be323e9c4f74370d40424796fb6 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 15 Feb 2023 12:06:53 +0000 Subject: [PATCH 05/24] chore: move encode_value and decode_value back inside Abi's impl --- crates/noirc_abi/src/lib.rs | 131 ++++++++++++++++++------------------ 1 file changed, 66 insertions(+), 65 deletions(-) diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 197c40088e4..ef9de220cc8 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -179,7 +179,7 @@ impl Abi { // First encode each input separately let encoded_input_map: BTreeMap> = try_btree_map(input_map, |(key, value)| { - encode_value(value, &key).map(|v| (key.clone(), v)) + Self::encode_value(value, &key).map(|v| (key.clone(), v)) })?; // Write input field elements into witness indices specified in `abi_witness_map`. @@ -216,7 +216,7 @@ impl Abi { return Err(AbiError::TypeMismatch { param, value }); } - encoded_inputs.extend(encode_value(value, ¶m.name)?); + encoded_inputs.extend(Self::encode_value(value, ¶m.name)?); } // Check that no extra witness values have been provided. @@ -230,6 +230,27 @@ impl Abi { Ok(encoded_inputs) } + fn encode_value(value: InputValue, param_name: &String) -> Result, AbiError> { + let mut encoded_value = Vec::new(); + match value { + InputValue::Field(elem) => encoded_value.push(elem), + InputValue::Vec(vec_elem) => encoded_value.extend(vec_elem), + InputValue::String(string) => { + let str_as_fields = + string.bytes().map(|byte| FieldElement::from_be_bytes_reduce(&[byte])); + encoded_value.extend(str_as_fields) + } + InputValue::Struct(object) => { + for (field_name, value) in object { + let new_name = format!("{param_name}.{field_name}"); + encoded_value.extend(Self::encode_value(value, &new_name)?) + } + } + InputValue::Undefined => return Err(AbiError::UndefinedInput(param_name.to_string())), + } + Ok(encoded_value) + } + /// Decode a `WitnessMap` into the types specified in the ABI. pub fn decode_from_witness( &self, @@ -244,7 +265,7 @@ impl Abi { witness_map[&witness_index] }); - decode_value(&mut param_witness_values.into_iter(), typ) + Self::decode_value(&mut param_witness_values.into_iter(), typ) .map(|input_value| (name.clone(), input_value)) .unwrap() }) @@ -269,79 +290,59 @@ impl Abi { let mut field_iterator = encoded_inputs.iter().cloned(); let mut decoded_inputs = BTreeMap::new(); for param in &self.parameters { - let decoded_value = decode_value(&mut field_iterator, ¶m.typ)?; + let decoded_value = Self::decode_value(&mut field_iterator, ¶m.typ)?; decoded_inputs.insert(param.name.to_owned(), decoded_value); } Ok(decoded_inputs) } -} -fn encode_value(value: InputValue, param_name: &String) -> Result, AbiError> { - let mut encoded_value = Vec::new(); - match value { - InputValue::Field(elem) => encoded_value.push(elem), - InputValue::Vec(vec_elem) => encoded_value.extend(vec_elem), - InputValue::String(string) => { - let str_as_fields = - string.bytes().map(|byte| FieldElement::from_be_bytes_reduce(&[byte])); - encoded_value.extend(str_as_fields) - } - InputValue::Struct(object) => { - for (field_name, value) in object { - let new_name = format!("{param_name}.{field_name}"); - encoded_value.extend(encode_value(value, &new_name)?) + fn decode_value( + field_iterator: &mut impl Iterator, + value_type: &AbiType, + ) -> Result { + // This function assumes that `field_iterator` contains enough `FieldElement`s in order to decode a `value_type` + // `Abi.decode` enforces that the encoded inputs matches the expected length defined by the ABI so this is safe. + let value = match value_type { + AbiType::Field | AbiType::Integer { .. } | AbiType::Boolean => { + let field_element = field_iterator.next().unwrap(); + + InputValue::Field(field_element) } - } - InputValue::Undefined => return Err(AbiError::UndefinedInput(param_name.to_string())), - } - Ok(encoded_value) -} + AbiType::Array { length, .. } => { + let field_elements: Vec = + field_iterator.take(*length as usize).collect(); -fn decode_value( - field_iterator: &mut impl Iterator, - value_type: &AbiType, -) -> Result { - // This function assumes that `field_iterator` contains enough `FieldElement`s in order to decode a `value_type` - // `Abi.decode` enforces that the encoded inputs matches the expected length defined by the ABI so this is safe. - let value = match value_type { - AbiType::Field | AbiType::Integer { .. } | AbiType::Boolean => { - let field_element = field_iterator.next().unwrap(); - - InputValue::Field(field_element) - } - AbiType::Array { length, .. } => { - let field_elements: Vec = field_iterator.take(*length as usize).collect(); + InputValue::Vec(field_elements) + } + AbiType::String { length } => { + let string_as_slice: Vec = field_iterator + .take(*length as usize) + .map(|e| { + let mut field_as_bytes = e.to_be_bytes(); + let char_byte = field_as_bytes.pop().unwrap(); // A character in a string is represented by a u8, thus we just want the last byte of the element + assert!(field_as_bytes.into_iter().all(|b| b == 0)); // Assert that the rest of the field element's bytes are empty + char_byte + }) + .collect(); + + let final_string = str::from_utf8(&string_as_slice).unwrap(); + + InputValue::String(final_string.to_owned()) + } + AbiType::Struct { fields, .. } => { + let mut struct_map = BTreeMap::new(); - InputValue::Vec(field_elements) - } - AbiType::String { length } => { - let string_as_slice: Vec = field_iterator - .take(*length as usize) - .map(|e| { - let mut field_as_bytes = e.to_be_bytes(); - let char_byte = field_as_bytes.pop().unwrap(); // A character in a string is represented by a u8, thus we just want the last byte of the element - assert!(field_as_bytes.into_iter().all(|b| b == 0)); // Assert that the rest of the field element's bytes are empty - char_byte - }) - .collect(); - - let final_string = str::from_utf8(&string_as_slice).unwrap(); - - InputValue::String(final_string.to_owned()) - } - AbiType::Struct { fields, .. } => { - let mut struct_map = BTreeMap::new(); + for (field_key, param_type) in fields { + let field_value = Self::decode_value(field_iterator, param_type)?; - for (field_key, param_type) in fields { - let field_value = decode_value(field_iterator, param_type)?; + struct_map.insert(field_key.to_owned(), field_value); + } - struct_map.insert(field_key.to_owned(), field_value); + InputValue::Struct(struct_map) } + }; - InputValue::Struct(struct_map) - } - }; - - Ok(value) + Ok(value) + } } From e301a6d7c3cd006f69179012873918df9c128e90 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 15 Feb 2023 12:20:39 +0000 Subject: [PATCH 06/24] chore: remove clone from `encode_to_witness` --- crates/noirc_abi/src/lib.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 21496e68c43..2710527638f 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -169,16 +169,12 @@ impl Abi { &self, input_map: &BTreeMap, ) -> Result, AbiError> { - // TODO: Avoid this clone - let mut input_map = input_map.clone(); - - // Remove the return value parameter - input_map.remove(MAIN_RETURN_NAME); + // TODO: add handling for unexpected inputs similarly to how we to in `encode_to_array`. // First encode each input separately let encoded_input_map: BTreeMap> = try_btree_map(input_map, |(key, value)| { - Self::encode_value(value, &key).map(|v| (key.clone(), v)) + Self::encode_value(value.clone(), key).map(|v| (key.clone(), v)) })?; // Write input field elements into witness indices specified in `abi_witness_map`. From 03d63c1bd06da137ba0b19305295abf1cdb95284 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 15 Feb 2023 12:24:40 +0000 Subject: [PATCH 07/24] chore: use try_btree_map instead of unwrapping --- crates/noirc_abi/src/lib.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 2710527638f..8e53e0e336d 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -251,20 +251,16 @@ impl Abi { &self, witness_map: &BTreeMap, ) -> Result, AbiError> { - let public_inputs_map = self - .parameters - .iter() - .map(|AbiParameter { name, typ, .. }| { + let public_inputs_map = + try_btree_map(self.parameters.clone(), |AbiParameter { name, typ, .. }| { let param_witness_values = - vecmap(self.param_witnesses[name].clone(), |witness_index| { + vecmap(self.param_witnesses[&name].clone(), |witness_index| { witness_map[&witness_index] }); - Self::decode_value(&mut param_witness_values.into_iter(), typ) + Self::decode_value(&mut param_witness_values.into_iter(), &typ) .map(|input_value| (name.clone(), input_value)) - .unwrap() - }) - .collect(); + })?; Ok(public_inputs_map) } From 067a711f9bfaf6db70045cc8f52430014ee6d65c Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 15 Feb 2023 13:35:40 +0000 Subject: [PATCH 08/24] feat: check for unexpected inputs when encoding a witness --- crates/noirc_abi/src/lib.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 8e53e0e336d..ffe72a3a7a3 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -169,7 +169,9 @@ impl Abi { &self, input_map: &BTreeMap, ) -> Result, AbiError> { - // TODO: add handling for unexpected inputs similarly to how we to in `encode_to_array`. + // TODO: add handling for missing inputs similarly to how we do in `encode_to_array`. + + self.check_for_unexpected_inputs(input_map)?; // First encode each input separately let encoded_input_map: BTreeMap> = @@ -197,32 +199,39 @@ impl Abi { self, inputs: &BTreeMap, ) -> Result, AbiError> { - let parameters = self.parameters.clone(); - let param_names: Vec<&String> = parameters.iter().map(|param| ¶m.name).collect(); let mut encoded_inputs = Vec::new(); - for param in self.parameters { + self.check_for_unexpected_inputs(inputs)?; + + for param in &self.parameters { let value = inputs .get(¶m.name) .ok_or_else(|| AbiError::MissingParam(param.name.to_owned()))? .clone(); if !value.matches_abi(¶m.typ) { - return Err(AbiError::TypeMismatch { param, value }); + return Err(AbiError::TypeMismatch { param: param.clone(), value }); } encoded_inputs.extend(Self::encode_value(value, ¶m.name)?); } - // Check that no extra witness values have been provided. - // Any missing values should be caught by the above for-loop so this only catches extra values. - if param_names.len() != inputs.len() { + Ok(encoded_inputs) + } + + /// Checks that no extra witness values have been provided. + fn check_for_unexpected_inputs( + &self, + inputs: &BTreeMap, + ) -> Result<(), AbiError> { + let param_names = self.parameter_names(); + if param_names.len() < inputs.len() { let unexpected_params: Vec = inputs.keys().filter(|param| !param_names.contains(param)).cloned().collect(); return Err(AbiError::UnexpectedParams(unexpected_params)); } - Ok(encoded_inputs) + Ok(()) } fn encode_value(value: InputValue, param_name: &String) -> Result, AbiError> { From 876a938d48ac40c24ea015a84095329d4990da63 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 15 Feb 2023 13:37:45 +0000 Subject: [PATCH 09/24] chore: use InputMap and WitnessMap types in ABI --- crates/noirc_abi/src/lib.rs | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index ffe72a3a7a3..2f3f88b7fdb 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -165,10 +165,7 @@ impl Abi { } /// Encode a set of inputs as described in the ABI into a `WitnessMap`. - pub fn encode_to_witness( - &self, - input_map: &BTreeMap, - ) -> Result, AbiError> { + pub fn encode_to_witness(&self, input_map: &InputMap) -> Result { // TODO: add handling for missing inputs similarly to how we do in `encode_to_array`. self.check_for_unexpected_inputs(input_map)?; @@ -195,10 +192,7 @@ impl Abi { } /// Encode a set of inputs as described in the ABI into a vector of `FieldElement`s. - pub fn encode_to_array( - self, - inputs: &BTreeMap, - ) -> Result, AbiError> { + pub fn encode_to_array(self, inputs: &InputMap) -> Result, AbiError> { let mut encoded_inputs = Vec::new(); self.check_for_unexpected_inputs(inputs)?; @@ -220,10 +214,7 @@ impl Abi { } /// Checks that no extra witness values have been provided. - fn check_for_unexpected_inputs( - &self, - inputs: &BTreeMap, - ) -> Result<(), AbiError> { + fn check_for_unexpected_inputs(&self, inputs: &InputMap) -> Result<(), AbiError> { let param_names = self.parameter_names(); if param_names.len() < inputs.len() { let unexpected_params: Vec = @@ -256,10 +247,7 @@ impl Abi { } /// Decode a `WitnessMap` into the types specified in the ABI. - pub fn decode_from_witness( - &self, - witness_map: &BTreeMap, - ) -> Result, AbiError> { + pub fn decode_from_witness(&self, witness_map: &WitnessMap) -> Result { let public_inputs_map = try_btree_map(self.parameters.clone(), |AbiParameter { name, typ, .. }| { let param_witness_values = @@ -275,10 +263,7 @@ impl Abi { } /// Decode a vector of `FieldElements` into the types specified in the ABI. - pub fn decode_from_array( - &self, - encoded_inputs: &[FieldElement], - ) -> Result, AbiError> { + pub fn decode_from_array(&self, encoded_inputs: &[FieldElement]) -> Result { let input_length: u32 = encoded_inputs.len().try_into().unwrap(); if input_length != self.field_count() { return Err(AbiError::UnexpectedInputLength { From 3262099db42b4d500a6a4b51af1ac3e831dc0aa6 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 15 Feb 2023 13:38:37 +0000 Subject: [PATCH 10/24] chore: update stale comment --- crates/noirc_abi/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 2f3f88b7fdb..e09a03259ba 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -176,7 +176,7 @@ impl Abi { Self::encode_value(value.clone(), key).map(|v| (key.clone(), v)) })?; - // Write input field elements into witness indices specified in `abi_witness_map`. + // Write input field elements into witness indices specified in `self.param_witnesses`. let witness_map = encoded_input_map .iter() .flat_map(|(param_name, encoded_param_fields)| { From dcb07a4261da0055d82318e2e9ec5301dd5769a6 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 12:22:58 +0000 Subject: [PATCH 11/24] chore: apply input validation checks on new encoding method --- crates/noirc_abi/src/lib.rs | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index e09a03259ba..bf2b31f0a89 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -166,15 +166,32 @@ impl Abi { /// Encode a set of inputs as described in the ABI into a `WitnessMap`. pub fn encode_to_witness(&self, input_map: &InputMap) -> Result { - // TODO: add handling for missing inputs similarly to how we do in `encode_to_array`. - self.check_for_unexpected_inputs(input_map)?; - // First encode each input separately - let encoded_input_map: BTreeMap> = - try_btree_map(input_map, |(key, value)| { - Self::encode_value(value.clone(), key).map(|v| (key.clone(), v)) - })?; + // First encode each input separately, performing any input validation. + let encoded_input_map: BTreeMap> = self + .to_btree_map() + .into_iter() + .filter(|(param_name, _)| param_name != MAIN_RETURN_NAME) + .map(|(param_name, expected_type)| { + let value = input_map + .get(¶m_name) + .ok_or_else(|| AbiError::MissingParam(param_name.clone()))? + .clone(); + + if !value.matches_abi(&expected_type) { + let missing_param = self + .parameters + .iter() + .find(|param| param.name == param_name) + .unwrap() + .clone(); + return Err(AbiError::TypeMismatch { param: missing_param, value }); + } + + Self::encode_value(value.clone(), ¶m_name).map(|v| (param_name, v)) + }) + .collect::>()?; // Write input field elements into witness indices specified in `self.param_witnesses`. let witness_map = encoded_input_map @@ -193,10 +210,9 @@ impl Abi { /// Encode a set of inputs as described in the ABI into a vector of `FieldElement`s. pub fn encode_to_array(self, inputs: &InputMap) -> Result, AbiError> { - let mut encoded_inputs = Vec::new(); - self.check_for_unexpected_inputs(inputs)?; + let mut encoded_inputs = Vec::new(); for param in &self.parameters { let value = inputs .get(¶m.name) From d004680585bfcf47cf9f4981c1e063f24e29d689 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 12:34:07 +0000 Subject: [PATCH 12/24] chore: clippy --- crates/noirc_abi/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index bf2b31f0a89..834ad9ae221 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -189,7 +189,7 @@ impl Abi { return Err(AbiError::TypeMismatch { param: missing_param, value }); } - Self::encode_value(value.clone(), ¶m_name).map(|v| (param_name, v)) + Self::encode_value(value, ¶m_name).map(|v| (param_name, v)) }) .collect::>()?; From c08d154e20cc9e40f3cce336f773f22e37b23b21 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 12:53:14 +0000 Subject: [PATCH 13/24] chore: remove unnecessary clone --- crates/noirc_evaluator/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index da39ffcd44a..cd0dd5a88c9 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -52,8 +52,7 @@ pub fn create_circuit( .public_abi() .parameter_names() .into_iter() - .cloned() - .flat_map(|param_name| evaluator.param_witnesses[¶m_name].clone()) + .flat_map(|param_name| evaluator.param_witnesses[param_name].clone()) .collect(); let witness_index = evaluator.current_witness_index(); From b71b95611d5c12f2480e91bcb9ef9185ce6fcfdc Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 13:42:45 +0000 Subject: [PATCH 14/24] feat: add smoke test for abi encoding and decoding --- crates/noirc_abi/src/input_parser/mod.rs | 1 + crates/noirc_abi/src/lib.rs | 57 ++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/crates/noirc_abi/src/input_parser/mod.rs b/crates/noirc_abi/src/input_parser/mod.rs index 680aef950e8..5ce9cbfd796 100644 --- a/crates/noirc_abi/src/input_parser/mod.rs +++ b/crates/noirc_abi/src/input_parser/mod.rs @@ -10,6 +10,7 @@ use crate::{Abi, AbiType}; /// This is what all formats eventually transform into /// For example, a toml file will parse into TomlTypes /// and those TomlTypes will be mapped to Value +#[cfg_attr(test, derive(PartialEq))] #[derive(Debug, Clone, Serialize)] pub enum InputValue { Field(FieldElement), diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 834ad9ae221..1f7f40d5b9d 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -350,3 +350,60 @@ pub fn decode_string_value(field_elements: &[FieldElement]) -> String { let final_string = str::from_utf8(&string_as_slice).unwrap(); final_string.to_owned() } + +#[cfg(test)] +mod test { + use std::collections::BTreeMap; + + use acvm::{acir::native_types::Witness, FieldElement}; + + use crate::{ + input_parser::InputValue, Abi, AbiParameter, AbiType, AbiVisibility, InputMap, + MAIN_RETURN_NAME, + }; + + #[test] + fn witness_encoding_roundtrip() { + let abi = Abi { + parameters: vec![ + AbiParameter { + name: "thing1".to_string(), + typ: AbiType::Array { length: 2, typ: Box::new(AbiType::Field) }, + visibility: AbiVisibility::Public, + }, + AbiParameter { + name: "thing2".to_string(), + typ: AbiType::Field, + visibility: AbiVisibility::Public, + }, + AbiParameter { + name: MAIN_RETURN_NAME.to_string(), + typ: AbiType::Field, + visibility: AbiVisibility::Public, + }, + ], + // Note that the return value shares a witness with `thing2` + param_witnesses: BTreeMap::from([ + ("thing1".to_string(), vec![Witness(1), Witness(2)]), + ("thing2".to_string(), vec![Witness(3)]), + (MAIN_RETURN_NAME.to_string(), vec![Witness(3)]), + ]), + }; + + // Note we omit return value from inputs + let inputs: InputMap = BTreeMap::from([ + ("thing1".to_string(), InputValue::Vec(vec![FieldElement::one(), FieldElement::one()])), + ("thing2".to_string(), InputValue::Field(FieldElement::zero())), + ]); + + let witness_map = abi.encode_to_witness(&inputs).unwrap(); + let reconstructed_inputs = abi.decode_from_witness(&witness_map).unwrap(); + + for (key, expected_value) in inputs { + assert_eq!(reconstructed_inputs[&key], expected_value); + } + + // We also decode the return value (we can do this immediately as we know it shares a witness with an input). + assert_eq!(reconstructed_inputs[MAIN_RETURN_NAME], reconstructed_inputs["thing2"]) + } +} From 0a5598c30e45d06e084196dfb1727cc013b546cc Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 13:46:42 +0000 Subject: [PATCH 15/24] chore: remove todo --- crates/noirc_evaluator/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index cd0dd5a88c9..42d8ddc8ba0 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -46,7 +46,6 @@ pub fn create_circuit( // First evaluate the main function evaluator.evaluate_main_alt(program.clone(), enable_logging, show_output)?; - // TODO: shrink this ungodly mess. let public_inputs = program .abi .public_abi() From f5200882cf40e5e9e9dab9820fd5c59b0852c841 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 14:16:30 +0000 Subject: [PATCH 16/24] chore: return abi from compile_circuit --- crates/noirc_driver/src/lib.rs | 11 ++--------- crates/noirc_evaluator/src/lib.rs | 19 +++++++++++-------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index b0a680f3d24..adc229e5859 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -181,16 +181,12 @@ impl Driver { local_crate.main_function().expect("cannot compile a program with no main function") }); - // Create ABI for main function - let func_meta = self.context.def_interner.function_meta(&main_function); - let mut abi = func_meta.into_abi(&self.context.def_interner); - let program = monomorphize(main_function, &self.context.def_interner); let blackbox_supported = acvm::default_is_black_box_supported(np_language.clone()); - let (circuit, param_witnesses) = + let (circuit, abi) = match create_circuit(program, np_language, blackbox_supported, show_ssa, show_output) { - Ok((circuit, param_witnesses)) => (circuit, param_witnesses), + Ok((circuit, abi)) => (circuit, abi), Err(err) => { // The FileId here will be the file id of the file with the main file // Errors will be shown at the call site without a stacktrace @@ -202,9 +198,6 @@ impl Driver { } }; - // TODO: Try to avoid this hack. - abi.param_witnesses = param_witnesses; - Ok(CompiledProgram { circuit, abi: Some(abi) }) } diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index 42d8ddc8ba0..0f706bb4617 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -10,7 +10,7 @@ use acvm::{ }; use errors::{RuntimeError, RuntimeErrorKind}; use iter_extended::btree_map; -use noirc_abi::AbiType; +use noirc_abi::{Abi, AbiType}; use noirc_frontend::monomorphization::ast::*; use ssa::{node, ssa_gen::IrGenerator}; use std::collections::BTreeMap; @@ -40,22 +40,25 @@ pub fn create_circuit( is_blackbox_supported: IsBlackBoxSupported, enable_logging: bool, show_output: bool, -) -> Result<(Circuit, BTreeMap>), RuntimeError> { +) -> Result<(Circuit, Abi), RuntimeError> { let mut evaluator = Evaluator::new(); // First evaluate the main function evaluator.evaluate_main_alt(program.clone(), enable_logging, show_output)?; - let public_inputs = program - .abi + let witness_index = evaluator.current_witness_index(); + + let mut abi = program.abi; + abi.param_witnesses = evaluator.param_witnesses; + + let public_inputs = abi + .clone() .public_abi() .parameter_names() .into_iter() - .flat_map(|param_name| evaluator.param_witnesses[param_name].clone()) + .flat_map(|param_name| abi.param_witnesses[param_name].clone()) .collect(); - let witness_index = evaluator.current_witness_index(); - let optimized_circuit = acvm::compiler::compile( Circuit { current_witness_index: witness_index, @@ -67,7 +70,7 @@ pub fn create_circuit( ) .map_err(|_| RuntimeErrorKind::Spanless(String::from("produced an acvm compile error")))?; - Ok((optimized_circuit, evaluator.param_witnesses)) + Ok((optimized_circuit, abi)) } impl Evaluator { From 609ffa7a1bda279b7211651276ec47fa486fa53e Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 14:21:42 +0000 Subject: [PATCH 17/24] chore: simplify returning `CompiledProgram` --- crates/noirc_driver/src/lib.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index adc229e5859..8643238e66a 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -184,21 +184,19 @@ impl Driver { let program = monomorphize(main_function, &self.context.def_interner); let blackbox_supported = acvm::default_is_black_box_supported(np_language.clone()); - let (circuit, abi) = - match create_circuit(program, np_language, blackbox_supported, show_ssa, show_output) { - Ok((circuit, abi)) => (circuit, abi), - Err(err) => { - // The FileId here will be the file id of the file with the main file - // Errors will be shown at the call site without a stacktrace - let file = err.location.map(|loc| loc.file); - let files = &self.context.file_manager; - let error = reporter::report(files, &err.into(), file, allow_warnings); - reporter::finish_report(error as u32)?; - return Err(ReportedError); - } - }; - Ok(CompiledProgram { circuit, abi: Some(abi) }) + match create_circuit(program, np_language, blackbox_supported, show_ssa, show_output) { + Ok((circuit, abi)) => Ok(CompiledProgram { circuit, abi: Some(abi) }), + Err(err) => { + // The FileId here will be the file id of the file with the main file + // Errors will be shown at the call site without a stacktrace + let file = err.location.map(|loc| loc.file); + let files = &self.context.file_manager; + let error = reporter::report(files, &err.into(), file, allow_warnings); + reporter::finish_report(error as u32)?; + return Err(ReportedError); + } + } } /// Returns a list of all functions in the current crate marked with #[test] From 50af42693ce085da4f71bccb21683308d64dc447 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 15:01:44 +0000 Subject: [PATCH 18/24] chore: clippy --- crates/noirc_driver/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index 8643238e66a..cfceb045e41 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -194,7 +194,7 @@ impl Driver { let files = &self.context.file_manager; let error = reporter::report(files, &err.into(), file, allow_warnings); reporter::finish_report(error as u32)?; - return Err(ReportedError); + Err(ReportedError) } } } From cc9a7ffd74f0ff9ebb80e43a5bd18b994d594b39 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 18:18:45 +0000 Subject: [PATCH 19/24] feat: return AbiError if witness value can't be found --- crates/noirc_abi/src/errors.rs | 5 +++++ crates/noirc_abi/src/lib.rs | 14 ++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/crates/noirc_abi/src/errors.rs b/crates/noirc_abi/src/errors.rs index fa35b4a9dba..f6f195694fe 100644 --- a/crates/noirc_abi/src/errors.rs +++ b/crates/noirc_abi/src/errors.rs @@ -1,4 +1,5 @@ use crate::{input_parser::InputValue, AbiParameter, AbiType}; +use acvm::acir::native_types::Witness; use thiserror::Error; #[derive(Debug, Error)] @@ -41,4 +42,8 @@ pub enum AbiError { UndefinedInput(String), #[error("ABI specifies an input of length {expected} but received input of length {actual}")] UnexpectedInputLength { expected: u32, actual: u32 }, + #[error( + "Could not read witness value at index {witness_index:?} (required for parameter \"{name}\")" + )] + MissingParamWitnessValue { name: String, witness_index: Witness }, } diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 1f7f40d5b9d..2d73f19e8ec 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -4,7 +4,7 @@ use std::{collections::BTreeMap, convert::TryInto, str}; use acvm::{acir::native_types::Witness, FieldElement}; use errors::AbiError; use input_parser::InputValue; -use iter_extended::{try_btree_map, vecmap}; +use iter_extended::{try_btree_map, try_vecmap, vecmap}; use serde::{Deserialize, Serialize}; // This is the ABI used to bridge the different TOML formats for the initial // witness, the partial witness generator and the interpreter. @@ -267,9 +267,15 @@ impl Abi { let public_inputs_map = try_btree_map(self.parameters.clone(), |AbiParameter { name, typ, .. }| { let param_witness_values = - vecmap(self.param_witnesses[&name].clone(), |witness_index| { - witness_map[&witness_index] - }); + try_vecmap(self.param_witnesses[&name].clone(), |witness_index| { + witness_map + .get(&witness_index) + .ok_or_else(|| AbiError::MissingParamWitnessValue { + name: name.clone(), + witness_index, + }) + .copied() + })?; Self::decode_value(&mut param_witness_values.into_iter(), &typ) .map(|input_value| (name.clone(), input_value)) From 4b948515e73cde58230cbc1d852767bbddf13ac9 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 18:44:51 +0000 Subject: [PATCH 20/24] chore: add documentation for Abi fields --- crates/noirc_abi/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 2d73f19e8ec..22695ec6459 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -125,7 +125,10 @@ impl AbiParameter { #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Abi { + /// An ordered list of the arguments to the program's `main` function, specifying their types and visibility. pub parameters: Vec, + /// A map from the ABI's parameters to the indices they are written to in the [`WitnessMap`]. + /// This defines how to convert between the [`InputMap`] and [`WitnessMap`]. pub param_witnesses: BTreeMap>, } From 8c4125f66ddb8cf65502df59a5334993be9f3179 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 19:20:10 +0000 Subject: [PATCH 21/24] fix: re-add `public_inputs` field to `Evaluator` --- crates/noirc_evaluator/src/lib.rs | 30 +++++++++---------- .../src/ssa/acir_gen/operations/return.rs | 1 + 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index 0f706bb4617..991e9408f10 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -10,7 +10,7 @@ use acvm::{ }; use errors::{RuntimeError, RuntimeErrorKind}; use iter_extended::btree_map; -use noirc_abi::{Abi, AbiType}; +use noirc_abi::{Abi, AbiType, AbiVisibility}; use noirc_frontend::monomorphization::ast::*; use ssa::{node, ssa_gen::IrGenerator}; use std::collections::BTreeMap; @@ -26,6 +26,10 @@ pub struct Evaluator { // creating the private/public inputs of the ABI. num_witnesses_abi_len: usize, param_witnesses: BTreeMap>, + // This is the list of witness indices which are linked to public inputs. + // Witnesses below `num_witnesses_abi_len` and not included in this set + // correspond to private inputs and must not be made public. + public_inputs: Vec, opcodes: Vec, } @@ -51,14 +55,7 @@ pub fn create_circuit( let mut abi = program.abi; abi.param_witnesses = evaluator.param_witnesses; - let public_inputs = abi - .clone() - .public_abi() - .parameter_names() - .into_iter() - .flat_map(|param_name| abi.param_witnesses[param_name].clone()) - .collect(); - + let public_inputs = evaluator.public_inputs.into_iter().collect(); let optimized_circuit = acvm::compiler::compile( Circuit { current_witness_index: witness_index, @@ -77,6 +74,7 @@ impl Evaluator { fn new() -> Self { Evaluator { num_witnesses_abi_len: 0, + public_inputs: Vec::new(), param_witnesses: BTreeMap::new(), // XXX: Barretenberg, reserves the first index to have value 0. // When we increment, we do not use this index at all. @@ -102,8 +100,7 @@ impl Evaluator { // an intermediate variable. let is_intermediate_variable = witness_index.as_usize() > self.num_witnesses_abi_len; - let is_public_input = - self.param_witnesses.values().flatten().any(|&index| index == witness_index); + let is_public_input = self.public_inputs.contains(&witness_index); !is_intermediate_variable && !is_public_input } @@ -154,6 +151,7 @@ impl Evaluator { name: &str, def: Definition, param_type: &AbiType, + param_visibility: &AbiVisibility, ir_gen: &mut IrGenerator, ) -> Result<(), RuntimeErrorKind> { let witnesses = match param_type { @@ -209,6 +207,9 @@ impl Evaluator { } }; + if param_visibility == &AbiVisibility::Public { + self.public_inputs.extend(witnesses.clone()); + } self.param_witnesses.insert(name.to_owned(), witnesses); Ok(()) @@ -304,11 +305,8 @@ impl Evaluator { for ((param_id, _, param_name, _), abi_param) in main_params.iter().zip(abi_params) { assert_eq!(param_name, &abi_param.name); let def = Definition::Local(*param_id); - self.param_to_var(param_name, def, &abi_param.typ, ir_gen).unwrap(); + self.param_to_var(param_name, def, &abi_param.typ, &abi_param.visibility, ir_gen) + .unwrap(); } - - // Store the number of witnesses used to represent the types - // in the ABI - self.num_witnesses_abi_len = self.current_witness_index as usize; } } diff --git a/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs b/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs index 7e20dad11e5..36efeb0de63 100644 --- a/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs +++ b/crates/noirc_evaluator/src/ssa/acir_gen/operations/return.rs @@ -54,6 +54,7 @@ pub(crate) fn evaluate( } witnesses.push(witness); } + evaluator.public_inputs.extend(witnesses.clone()); evaluator.param_witnesses.insert(MAIN_RETURN_NAME.to_owned(), witnesses); } From da6ee4289b09ccfb5e334458a80be680c76e75e2 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 19:24:37 +0000 Subject: [PATCH 22/24] chore: remove stale code from BTreeSet experiment --- crates/noirc_evaluator/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index 991e9408f10..26bb33ac496 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -55,12 +55,11 @@ pub fn create_circuit( let mut abi = program.abi; abi.param_witnesses = evaluator.param_witnesses; - let public_inputs = evaluator.public_inputs.into_iter().collect(); let optimized_circuit = acvm::compiler::compile( Circuit { current_witness_index: witness_index, opcodes: evaluator.opcodes, - public_inputs: PublicInputs(public_inputs), + public_inputs: PublicInputs(evaluator.public_inputs), }, np_language, is_blackbox_supported, From 2117c5bb61082bfb03eba0640ac758d0b981ca05 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 19:53:38 +0000 Subject: [PATCH 23/24] chore: fix merge issues --- crates/nargo/src/cli/execute_cmd.rs | 2 +- crates/nargo/src/cli/prove_cmd.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/nargo/src/cli/execute_cmd.rs b/crates/nargo/src/cli/execute_cmd.rs index ad5c8d2181c..25c27d75bdb 100644 --- a/crates/nargo/src/cli/execute_cmd.rs +++ b/crates/nargo/src/cli/execute_cmd.rs @@ -65,7 +65,7 @@ pub(crate) fn execute_program( // Solve the remaining witnesses let solved_witness = solve_witness(compiled_program, inputs_map)?; - let public_abi = compiled_program.abi.as_ref().unwrap().clone().public_abi(); + let public_abi = compiled_program.abi.clone().public_abi(); let public_inputs = public_abi.decode_from_witness(&solved_witness)?; let return_value = public_inputs.get(MAIN_RETURN_NAME).cloned(); diff --git a/crates/nargo/src/cli/prove_cmd.rs b/crates/nargo/src/cli/prove_cmd.rs index 0203977af06..c788f8ed162 100644 --- a/crates/nargo/src/cli/prove_cmd.rs +++ b/crates/nargo/src/cli/prove_cmd.rs @@ -54,7 +54,7 @@ pub fn prove_with_path>( let (_, solved_witness) = execute_program(&compiled_program, &inputs_map)?; // Write public inputs into Verifier.toml - let public_abi = compiled_program.abi.as_ref().unwrap().clone().public_abi(); + let public_abi = compiled_program.abi.clone().public_abi(); let public_inputs = public_abi.decode_from_witness(&solved_witness)?; write_inputs_to_file(&public_inputs, &program_dir, VERIFIER_INPUT_FILE, Format::Toml)?; From 4f0574aeecfce3aaea6ffff84c7fbb48ec360013 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 16 Feb 2023 20:22:35 +0000 Subject: [PATCH 24/24] fix: re-add deleted `num_witnesses_abi_len` init --- crates/noirc_evaluator/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index 26bb33ac496..9feaee396df 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -307,5 +307,9 @@ impl Evaluator { self.param_to_var(param_name, def, &abi_param.typ, &abi_param.visibility, ir_gen) .unwrap(); } + + // Store the number of witnesses used to represent the types + // in the ABI + self.num_witnesses_abi_len = self.current_witness_index as usize; } }