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

@@ -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<WitnessMap, CliError> {
     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."))
             }

crates/nargo/src/cli/mod.rs

@@ -1,15 +1,11 @@
-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;
 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<FieldElement>,
-) -> (PublicInputs, Vec<FieldElement>) {
-    // 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<P: AsRef<Path>>(path: P) -> Result<Vec<u8>, CliError> {
     let hex_data: Vec<_> =
         std::fs::read(&path).map_err(|_| CliError::PathNotValid(path.as_ref().to_path_buf()))?;

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<P: AsRef<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 {

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<P: AsRef<Path>>(
 }
 
 pub(crate) fn verify_proof(
-    mut compiled_program: CompiledProgram,
+    compiled_program: CompiledProgram,
     public_inputs_map: InputMap,
     proof: &[u8],
     verification_key: Vec<u8>,
 ) -> Result<bool, CliError> {
     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<FieldElement> = 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,
     );

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}\")"
     )]

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<WitnessMap, AbiError> {
+    pub fn encode(&self, input_map: &InputMap, skip_output: bool) -> Result<WitnessMap, AbiError> {
         self.check_for_unexpected_inputs(input_map)?;
 
         // First encode each input separately, performing any input validation.
         let encoded_input_map: BTreeMap<String, Vec<FieldElement>> = 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(&param_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<Vec<FieldElement>, AbiError> {
-        self.check_for_unexpected_inputs(inputs)?;
-
-        let mut encoded_inputs = Vec::new();
-        for param in &self.parameters {
-            let value = inputs
-                .get(&param.name)
-                .ok_or_else(|| AbiError::MissingParam(param.name.to_owned()))?
-                .clone();
-
-            if !value.matches_abi(&param.typ) {
-                return Err(AbiError::TypeMismatch { param: param.clone(), value });
-            }
-
-            encoded_inputs.extend(Self::encode_value(value, &param.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<InputMap, AbiError> {
+    pub fn decode(&self, witness_map: &WitnessMap) -> Result<InputMap, AbiError> {
         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<InputMap, AbiError> {
-        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, &param.typ)?;
-
-            decoded_inputs.insert(param.name.to_owned(), decoded_value);
-        }
-        Ok(decoded_inputs)
-    }
-
     fn decode_value(
         field_iterator: &mut impl Iterator<Item = FieldElement>,
         value_type: &AbiType,

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<Witness>,
+    public_inputs: BTreeSet<Witness>,
     opcodes: Vec<AcirOpcode>,
 }
 
@@ -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.