Returned values from main are set to public (Deprecated)

crates/nargo/src/cli/prove_cmd.rs

@@ -1,20 +1,21 @@
-use std::{collections::BTreeMap, path::PathBuf};
-
-use acvm::acir::native_types::Witness;
-use acvm::FieldElement;
-use acvm::ProofSystemCompiler;
-use acvm::{GateResolution, PartialWitnessGenerator};
 use clap::ArgMatches;
-use noirc_abi::errors::AbiError;
-use noirc_abi::input_parser::{Format, InputValue};
-use std::path::Path;
+use std::{
+    collections::{BTreeMap, HashSet},
+    path::{Path, PathBuf},
+};
 
-use crate::errors::CliError;
+use acvm::acir::{circuit::PublicInputs, native_types::Witness, FieldElement};
+use acvm::{GateResolution, PartialWitnessGenerator, ProofSystemCompiler};
+use noirc_abi::{
+    errors::AbiError,
+    input_parser::{Format, InputValue},
+};
 
 use super::{
     create_named_dir, read_inputs_from_file, write_inputs_to_file, write_to_file, PROOFS_DIR,
     PROOF_EXT, PROVER_INPUT_FILE, VERIFIER_INPUT_FILE,
 };
+use crate::errors::CliError;
 
 pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> {
     let args = args.subcommand_matches("prove").unwrap();
@@ -127,10 +128,18 @@ pub fn prove_with_path<P: AsRef<Path>>(
     show_ssa: bool,
     allow_warnings: bool,
 ) -> Result<PathBuf, CliError> {
-    let (compiled_program, solved_witness) =
+    let (mut compiled_program, solved_witness) =
         compile_circuit_and_witness(program_dir, show_ssa, allow_warnings)?;
 
     let backend = crate::backends::ConcreteBackend;
+
+    // Since the public outputs are added into 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, for creating a proof, we remove these duplicates.
+    compiled_program.circuit.public_inputs =
+        dedup_public_input_indices(compiled_program.circuit.public_inputs);
+
     let proof = backend.prove_with_meta(compiled_program.circuit, solved_witness);
 
     let mut proof_path = create_named_dir(proof_dir.as_ref(), "proof");
@@ -145,3 +154,34 @@ pub fn prove_with_path<P: AsRef<Path>>(
 
     Ok(proof_path)
 }
+
+// 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 = HashSet::new();
+
+    for (index, value) in indices.0.iter().zip(values) {
+        if !already_seen_public_indices.contains(&index) {
+            already_seen_public_indices.insert(index);
+            public_inputs_without_duplicates.push(value)
+        }
+    }
+
+    (
+        PublicInputs(already_seen_public_indices.into_iter().cloned().collect()),
+        public_inputs_without_duplicates,
+    )
+}

crates/nargo/src/cli/verify_cmd.rs

@@ -1,13 +1,19 @@
-use super::compile_cmd::compile_circuit;
-use super::{read_inputs_from_file, PROOFS_DIR, PROOF_EXT, VERIFIER_INPUT_FILE};
-use crate::errors::CliError;
-use acvm::ProofSystemCompiler;
 use clap::ArgMatches;
-use noirc_abi::errors::AbiError;
-use noirc_abi::input_parser::{Format, InputValue};
-use noirc_driver::CompiledProgram;
 use std::{collections::BTreeMap, path::Path, path::PathBuf};
 
+use acvm::ProofSystemCompiler;
+use noirc_abi::{
+    errors::AbiError,
+    input_parser::{Format, InputValue},
+};
+use noirc_driver::CompiledProgram;
+
+use super::{
+    compile_cmd::compile_circuit, prove_cmd::dedup_public_input_indices_values,
+    read_inputs_from_file, PROOFS_DIR, PROOF_EXT, VERIFIER_INPUT_FILE,
+};
+use crate::errors::CliError;
+
 pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> {
     let args = args.subcommand_matches("verify").unwrap();
 
@@ -56,7 +62,7 @@ pub fn verify_with_path<P: AsRef<Path>>(
 }
 
 fn verify_proof(
-    compiled_program: CompiledProgram,
+    mut compiled_program: CompiledProgram,
     public_inputs: BTreeMap<String, InputValue>,
     proof: Vec<u8>,
 ) -> Result<bool, CliError> {
@@ -68,8 +74,14 @@ fn verify_proof(
         _ => 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 backend = crate::backends::ConcreteBackend;
-    let valid_proof = backend.verify_from_cs(&proof, public_inputs, compiled_program.circuit);
+    let valid_proof = backend.verify_from_cs(&proof, dedup_public_values, compiled_program.circuit);
 
     Ok(valid_proof)
 }

crates/nargo/tests/test_data/hash_to_field/Prover.toml

@@ -1,2 +1 @@
 input = "1"
-return = "0x25cebc29ded2fa515a937e2b5f674e3026c012e5b57f8a48d7dce6b7d274f9d9"

crates/nargo/tests/test_data/main_return/Prover.toml

@@ -1,2 +1 @@
-return = ""
 x = "8"

crates/nargo/tests/test_data/simple_shield/Prover.toml

@@ -5,11 +5,7 @@ index = "0"
 note_hash_path = [
     "0x0000000000000000000000000000000000000000000000000000000000000000",
     "0x0e4223f3925f98934393c74975142bd73079ab0621f4ee133cee050a3c194f1a",
-    "0x2fd7bb412155bf8693a3bd2a3e7581a679c95c68a052f835dddca85fa1569a40"
+    "0x2fd7bb412155bf8693a3bd2a3e7581a679c95c68a052f835dddca85fa1569a40",
 ]
 to_pubkey_x = "0x0000000000000000000000000000000000000000000000000000000000000001"
 to_pubkey_y = "0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c"
-return = [
-    "0x1fa077bf4c787f858e0f3a7fa031ff11f200a0ccc6e549c6af98ec96800af340",
-    "0x0c25956c07e2277e75f1ca10bae32c29f24b27f25625fe9258cec29dc90651dd"
-]

crates/nargo/tests/test_data/struct_inputs/Prover.toml

@@ -1,5 +1,4 @@
 x = "5"
-return = ""
 
 [y]
 foo = "5"
@@ -12,4 +11,4 @@ array = [0, 1]
 [a]
 [a.bar_struct]
 val = "1"
-array = [0, 1]
+array = [0, 1]

crates/nargo/tests/test_data/to_bytes/Prover.toml

@@ -1,2 +1 @@
 x = "2040124"
-return = [0x3c, 0x21, 0x1f, 0x00]

crates/noirc_abi/src/lib.rs

@@ -141,12 +141,23 @@ impl Abi {
     pub fn encode(
         self,
         inputs: &BTreeMap<String, InputValue>,
-        allow_undefined_return: bool,
+        skip_output: bool,
     ) -> Result<Vec<FieldElement>, AbiError> {
-        let param_names = self.parameter_names();
+        // 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.
+        // TODO Adding a `public outputs` field into acir and
+        // TODO the ABI will clean up this logic
+        // TODO For prosperity; the prover does not know about a `return` value
+        // TODO 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(&&param.name));
+        let param_names: Vec<&String> = parameters.clone().map(|param| &param.name).collect();
         let mut encoded_inputs = Vec::new();
 
-        for param in self.parameters.iter() {
+        for param in parameters {
             let value = inputs
                 .get(&param.name)
                 .ok_or_else(|| AbiError::MissingParam(param.name.to_owned()))?
@@ -156,26 +167,6 @@ impl Abi {
                 return Err(AbiError::TypeMismatch { param: param.to_owned(), value });
             }
 
-            // As the circuit calculates the return value in the process of calculating rest of the witnesses
-            // it's not absolutely necessary to provide them as inputs. We then tolerate an undefined value for
-            // the return value input and just skip it.
-            if allow_undefined_return
-                && param.name == MAIN_RETURN_NAME
-                && matches!(value, InputValue::Undefined)
-            {
-                let return_witness_len = param.typ.field_count();
-
-                // We do not support undefined arrays for now - TODO
-                if return_witness_len != 1 {
-                    return Err(AbiError::Generic(
-                        "Values of array returned from main must be specified".to_string(),
-                    ));
-                } else {
-                    // This assumes that the return value is at the end of the ABI, otherwise values will be misaligned.
-                    continue;
-                }
-            }
-
             encoded_inputs.extend(Self::encode_value(value, &param.name)?);
         }
 
@@ -221,7 +212,6 @@ impl Abi {
 
         let mut index = 0;
         let mut decoded_inputs = BTreeMap::new();
-
         for param in &self.parameters {
             let (next_index, decoded_value) =
                 Self::decode_value(index, encoded_inputs, &param.typ)?;

crates/noirc_evaluator/src/lib.rs

@@ -233,6 +233,17 @@ impl Evaluator {
         let main = igen.program.main();
         let main_params = std::mem::take(&mut main.parameters);
         let abi_params = std::mem::take(&mut igen.program.abi.parameters);
+
+        // Remove the return type from the parameters
+        // Since this is not in the main functions parameters.
+        //
+        // TODO(See Issue633) regarding adding a `return_type` field to the ABI struct
+        // TODO(See Issue631) regarding making the return keyword reserved
+        let abi_params: Vec<_> = abi_params
+            .into_iter()
+            .filter(|param| param.name != noirc_abi::MAIN_RETURN_NAME)
+            .collect();
+
         assert_eq!(main_params.len(), abi_params.len());
 
         for ((param_id, _, param_name1, _), abi_param) in main_params.iter().zip(abi_params) {

crates/noirc_evaluator/src/ssa/acir_gen.rs

@@ -230,6 +230,35 @@ impl Acir {
                     todo!("dynamic arrays are not implemented yet");
                 }
             }
+            Operation::SetPub(node_ids, _) => {
+                for node_id in node_ids {
+                    // An array produces a single node_id
+                    // We therefore need to check if the node_id is referring to an array
+                    // and deference to get the elements
+                    let objects = match Memory::deref(ctx, *node_id) {
+                        Some(a) => {
+                            let array = &ctx.mem[a];
+                            self.load_array(array, false, evaluator)
+                        }
+                        None => vec![self.substitute(*node_id, evaluator, ctx)],
+                    };
+
+                    for mut object in objects {
+                        // TODO(Open issue): We want to semantically specify that
+                        // TODO you cannot return constant values as public outputs.
+                        // TODO: in that case, you should just have a constant.
+                        // TODO: For now, we will support this usecase.
+                        let witness = if object.expression.is_const() {
+                            evaluator.create_intermediate_variable(object.expression)
+                        } else {
+                            object.generate_witness(evaluator)
+                        };
+                        evaluator.public_inputs.push(witness);
+                    }
+                }
+
+                InternalVar::default()
+            }
         };
         output.id = Some(ins.id);
         self.arith_cache.insert(ins.id, output);

crates/noirc_evaluator/src/ssa/code_gen.rs

@@ -366,6 +366,17 @@ impl IRGenerator {
         Ok(Value::dummy())
     }
 
+    fn codegen_setpub(
+        &mut self,
+        expr: &Expression,
+        location: noirc_errors::Location,
+    ) -> Result<Value, RuntimeError> {
+        let node_ids = self.codegen_expression(expr)?.to_node_ids();
+        let operation = Operation::SetPub(node_ids, Some(location));
+        self.context.new_instruction(operation, ObjectType::NotAnObject)?;
+        Ok(Value::dummy())
+    }
+
     /// Bind the given DefinitionId to the given Value. This will flatten the Value as needed,
     /// expanding each field of the value to a new variable.
     fn bind_id(&mut self, id: DefinitionId, value: Value, name: &str) -> Result<(), RuntimeError> {
@@ -601,6 +612,7 @@ impl IRGenerator {
                 self.codegen_expression(expr.as_ref())?;
                 Ok(Value::dummy())
             }
+            Expression::SetPub(expr, location) => self.codegen_setpub(expr, *location),
         }
     }
 

crates/noirc_evaluator/src/ssa/context.rs

@@ -236,6 +236,13 @@ impl SsaContext {
                 let call = self.node_to_string(*call_instruction);
                 format!("result {index} of {call}")
             }
+            Operation::SetPub(node_ids, _) => {
+                let mut s = "setpub ".to_string();
+                for node_id in node_ids {
+                    s = format!("{}, {}", s, self.node_to_string(*node_id),);
+                }
+                s
+            }
         }
     }
 

crates/noirc_evaluator/src/ssa/inline.rs

@@ -408,7 +408,7 @@ impl node::Operation {
     ) {
         match self {
             //default way to handle arrays during inlining; we map arrays using the stack_frame
-            Operation::Binary(_) | Operation::Constrain(..) | Operation::Intrinsic(_,_)
+            Operation::Binary(_) | Operation::Constrain(..) | Operation::Intrinsic(_,_) | Operation::SetPub(_,_)
             => {
                 self.map_id_mut(|id| {
                     if let Some(a) = Memory::deref(ctx, id) {

crates/noirc_evaluator/src/ssa/integer.rs

@@ -482,6 +482,7 @@ fn get_max_value(ins: &Instruction, max_map: &mut HashMap<NodeId, BigUint>) -> B
             unreachable!("Functions must have been inlined before checking for overflows")
         }
         Operation::Intrinsic(opcode, _) => opcode.get_max_value(),
+        Operation::SetPub(_, _) => BigUint::zero(),
     };
 
     if ins.res_type == ObjectType::NativeField {