From fecd32cc27b552eb47681618ba44894c635c7f8c Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Fri, 17 Feb 2023 09:43:37 +0000 Subject: [PATCH] feat(abi)!: merge both abi encoding/decoding methods (#862) * feat(abi)!: merge both abi encoding/decoding methods feat: deduplicate public inputs in evaluator * chore: fmt --- crates/nargo/src/cli/execute_cmd.rs | 4 +-- crates/nargo/src/cli/mod.rs | 44 +------------------------- crates/nargo/src/cli/prove_cmd.rs | 15 +++------ crates/nargo/src/cli/verify_cmd.rs | 18 +++++------ crates/noirc_abi/src/errors.rs | 2 -- crates/noirc_abi/src/lib.rs | 49 +++-------------------------- crates/noirc_evaluator/src/lib.rs | 9 +++--- 7 files changed, 24 insertions(+), 117 deletions(-) diff --git a/crates/nargo/src/cli/execute_cmd.rs b/crates/nargo/src/cli/execute_cmd.rs index 43784683656..d9291b00876 100644 --- a/crates/nargo/src/cli/execute_cmd.rs +++ b/crates/nargo/src/cli/execute_cmd.rs @@ -76,7 +76,7 @@ pub(crate) fn execute_program( let solved_witness = solve_witness(compiled_program, inputs_map)?; let public_abi = compiled_program.abi.clone().public_abi(); - let public_inputs = public_abi.decode_from_witness(&solved_witness)?; + let public_inputs = public_abi.decode(&solved_witness)?; let return_value = public_inputs.get(MAIN_RETURN_NAME).cloned(); Ok((return_value, solved_witness)) @@ -87,7 +87,7 @@ pub(crate) fn solve_witness( input_map: &InputMap, ) -> Result { let mut solved_witness = - compiled_program.abi.encode_to_witness(input_map).map_err(|error| match error { + compiled_program.abi.encode(input_map, true).map_err(|error| match error { AbiError::UndefinedInput(_) => { CliError::Generic(format!("{error} in the {PROVER_INPUT_FILE}.toml file.")) } diff --git a/crates/nargo/src/cli/mod.rs b/crates/nargo/src/cli/mod.rs index 9c9d8b151af..8c7d354f040 100644 --- a/crates/nargo/src/cli/mod.rs +++ b/crates/nargo/src/cli/mod.rs @@ -1,7 +1,4 @@ -use acvm::{ - acir::circuit::{Circuit, PublicInputs}, - hash_constraint_system, FieldElement, ProofSystemCompiler, -}; +use acvm::{acir::circuit::Circuit, hash_constraint_system, ProofSystemCompiler}; pub use check_cmd::check_from_path; use clap::{Args, Parser, Subcommand}; use const_format::formatcp; @@ -9,7 +6,6 @@ use noirc_abi::{input_parser::Format, Abi, InputMap}; use noirc_driver::Driver; use noirc_frontend::graph::{CrateName, CrateType}; use std::{ - collections::{HashMap, HashSet}, fs::File, io::Write, path::{Path, PathBuf}, @@ -184,44 +180,6 @@ fn path_to_stdlib() -> PathBuf { dirs::config_dir().unwrap().join("noir-lang").join("std/src") } -// Removes duplicates from the list of public input witnesses -fn dedup_public_input_indices(indices: PublicInputs) -> PublicInputs { - let duplicates_removed: HashSet<_> = indices.0.into_iter().collect(); - PublicInputs(duplicates_removed.into_iter().collect()) -} - -// Removes duplicates from the list of public input witnesses and the -// associated list of duplicate values. -pub(crate) fn dedup_public_input_indices_values( - indices: PublicInputs, - values: Vec, -) -> (PublicInputs, Vec) { - // Assume that the public input index lists and the values contain duplicates - assert_eq!(indices.0.len(), values.len()); - - let mut public_inputs_without_duplicates = Vec::new(); - let mut already_seen_public_indices = HashMap::new(); - - for (index, value) in indices.0.iter().zip(values) { - match already_seen_public_indices.get(index) { - Some(expected_value) => { - // The index has already been added - // so lets check that the values already inserted is equal to the value, we wish to insert - assert_eq!(*expected_value, value, "witness index {index:?} does not have a canonical map. The expected value is {expected_value}, the received value is {value}.") - } - None => { - already_seen_public_indices.insert(*index, value); - public_inputs_without_duplicates.push(value) - } - } - } - - ( - PublicInputs(already_seen_public_indices.keys().copied().collect()), - public_inputs_without_duplicates, - ) -} - pub fn load_hex_data>(path: P) -> Result, CliError> { let hex_data: Vec<_> = std::fs::read(&path).map_err(|_| CliError::PathNotValid(path.as_ref().to_path_buf()))?; diff --git a/crates/nargo/src/cli/prove_cmd.rs b/crates/nargo/src/cli/prove_cmd.rs index a6eed1ece41..f149301e108 100644 --- a/crates/nargo/src/cli/prove_cmd.rs +++ b/crates/nargo/src/cli/prove_cmd.rs @@ -5,8 +5,8 @@ use clap::Args; use noirc_abi::input_parser::Format; use super::{ - create_named_dir, dedup_public_input_indices, fetch_pk_and_vk, read_inputs_from_file, - write_inputs_to_file, write_to_file, NargoConfig, + create_named_dir, fetch_pk_and_vk, read_inputs_from_file, write_inputs_to_file, write_to_file, + NargoConfig, }; use crate::{ cli::{execute_cmd::execute_program, verify_cmd::verify_proof}, @@ -92,17 +92,12 @@ pub fn prove_with_path>( // Write public inputs into Verifier.toml let public_abi = compiled_program.abi.clone().public_abi(); - let public_inputs = public_abi.decode_from_witness(&solved_witness)?; + let public_inputs = public_abi.decode(&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. - // We keep the duplicates for when one is encoding the return values into the Verifier.toml, - // however we must remove these duplicates when creating a proof. - let mut prover_circuit = compiled_program.circuit.clone(); - prover_circuit.public_inputs = dedup_public_input_indices(prover_circuit.public_inputs); - let backend = crate::backends::ConcreteBackend; - let proof = backend.prove_with_pk(prover_circuit, solved_witness, proving_key); + let proof = + backend.prove_with_pk(compiled_program.circuit.clone(), solved_witness, proving_key); println!("Proof successfully created"); if check_proof { diff --git a/crates/nargo/src/cli/verify_cmd.rs b/crates/nargo/src/cli/verify_cmd.rs index 9b81d7dfdc1..cb062077e78 100644 --- a/crates/nargo/src/cli/verify_cmd.rs +++ b/crates/nargo/src/cli/verify_cmd.rs @@ -1,12 +1,12 @@ use super::{ - compile_cmd::compile_circuit, dedup_public_input_indices_values, fetch_pk_and_vk, - load_hex_data, read_inputs_from_file, InputMap, NargoConfig, + compile_cmd::compile_circuit, fetch_pk_and_vk, load_hex_data, read_inputs_from_file, InputMap, + NargoConfig, }; use crate::{ constants::{PROOFS_DIR, PROOF_EXT, TARGET_DIR, VERIFIER_INPUT_FILE}, errors::CliError, }; -use acvm::ProofSystemCompiler; +use acvm::{FieldElement, ProofSystemCompiler}; use clap::Args; use noirc_abi::errors::AbiError; use noirc_abi::input_parser::Format; @@ -87,30 +87,26 @@ pub fn verify_with_path>( } pub(crate) fn verify_proof( - mut compiled_program: CompiledProgram, + compiled_program: CompiledProgram, public_inputs_map: InputMap, proof: &[u8], verification_key: Vec, ) -> Result { let public_abi = compiled_program.abi.public_abi(); let public_inputs = - public_abi.encode_to_array(&public_inputs_map).map_err(|error| match error { + public_abi.encode(&public_inputs_map, false).map_err(|error| match error { AbiError::UndefinedInput(_) => { CliError::Generic(format!("{error} in the {VERIFIER_INPUT_FILE}.toml file.")) } _ => CliError::from(error), })?; - // Similarly to when proving -- we must remove the duplicate public witnesses which - // can be present because a public input can also be added as a public output. - let (dedup_public_indices, dedup_public_values) = - dedup_public_input_indices_values(compiled_program.circuit.public_inputs, public_inputs); - compiled_program.circuit.public_inputs = dedup_public_indices; + let public_inputs_vec: Vec = public_inputs.values().copied().collect(); let backend = crate::backends::ConcreteBackend; let valid_proof = backend.verify_with_vk( proof, - dedup_public_values, + public_inputs_vec, compiled_program.circuit, verification_key, ); diff --git a/crates/noirc_abi/src/errors.rs b/crates/noirc_abi/src/errors.rs index f6f195694fe..55a3683b347 100644 --- a/crates/noirc_abi/src/errors.rs +++ b/crates/noirc_abi/src/errors.rs @@ -40,8 +40,6 @@ pub enum AbiError { MissingParam(String), #[error("Input value `{0}` is not defined")] 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}\")" )] diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs index 22695ec6459..699e9561169 100644 --- a/crates/noirc_abi/src/lib.rs +++ b/crates/noirc_abi/src/lib.rs @@ -1,5 +1,5 @@ #![forbid(unsafe_code)] -use std::{collections::BTreeMap, convert::TryInto, str}; +use std::{collections::BTreeMap, str}; use acvm::{acir::native_types::Witness, FieldElement}; use errors::AbiError; @@ -168,14 +168,14 @@ 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 { + pub fn encode(&self, input_map: &InputMap, skip_output: bool) -> Result { self.check_for_unexpected_inputs(input_map)?; // 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) + .filter(|(param_name, _)| !skip_output || param_name != MAIN_RETURN_NAME) .map(|(param_name, expected_type)| { let value = input_map .get(¶m_name) @@ -211,27 +211,6 @@ impl Abi { Ok(witness_map) } - /// 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> { - self.check_for_unexpected_inputs(inputs)?; - - let mut encoded_inputs = Vec::new(); - 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.clone(), value }); - } - - encoded_inputs.extend(Self::encode_value(value, ¶m.name)?); - } - - Ok(encoded_inputs) - } - /// Checks that no extra witness values have been provided. fn check_for_unexpected_inputs(&self, inputs: &InputMap) -> Result<(), AbiError> { let param_names = self.parameter_names(); @@ -266,7 +245,7 @@ impl Abi { } /// Decode a `WitnessMap` into the types specified in the ABI. - pub fn decode_from_witness(&self, witness_map: &WitnessMap) -> Result { + pub fn decode(&self, witness_map: &WitnessMap) -> Result { let public_inputs_map = try_btree_map(self.parameters.clone(), |AbiParameter { name, typ, .. }| { let param_witness_values = @@ -287,26 +266,6 @@ impl Abi { Ok(public_inputs_map) } - /// Decode a vector of `FieldElements` into the types specified in the ABI. - 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 { - actual: input_length, - expected: self.field_count(), - }); - } - - 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)?; - - decoded_inputs.insert(param.name.to_owned(), decoded_value); - } - Ok(decoded_inputs) - } - fn decode_value( field_iterator: &mut impl Iterator, value_type: &AbiType, diff --git a/crates/noirc_evaluator/src/lib.rs b/crates/noirc_evaluator/src/lib.rs index 9feaee396df..9a094661074 100644 --- a/crates/noirc_evaluator/src/lib.rs +++ b/crates/noirc_evaluator/src/lib.rs @@ -13,7 +13,7 @@ use iter_extended::btree_map; use noirc_abi::{Abi, AbiType, AbiVisibility}; use noirc_frontend::monomorphization::ast::*; use ssa::{node, ssa_gen::IrGenerator}; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; pub struct Evaluator { // Why is this not u64? @@ -29,7 +29,7 @@ pub struct Evaluator { // 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, + public_inputs: BTreeSet, opcodes: Vec, } @@ -55,11 +55,12 @@ 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(evaluator.public_inputs), + public_inputs: PublicInputs(public_inputs), }, np_language, is_blackbox_supported, @@ -73,7 +74,7 @@ impl Evaluator { fn new() -> Self { Evaluator { num_witnesses_abi_len: 0, - public_inputs: Vec::new(), + public_inputs: BTreeSet::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.