chore(cli): Forward nargo execute to noir_artifact_cli (#7406)

Co-authored-by: Tom French <tom@tomfren.ch>

Author: aakoshh

acvm-repo/acir/src/native_types/witness_stack.rs

@@ -12,6 +12,9 @@ use super::WitnessMap;
 enum SerializationError {
     #[error(transparent)]
     Deflate(#[from] std::io::Error),
+
+    #[error(transparent)]
+    BincodeError(#[from] bincode::Error),
 }
 
 #[derive(Debug, Error)]
@@ -57,26 +60,35 @@ impl<F> From<WitnessMap<F>> for WitnessStack<F> {
     }
 }
 
-impl<F: Serialize> TryFrom<WitnessStack<F>> for Vec<u8> {
+impl<F: Serialize> TryFrom<&WitnessStack<F>> for Vec<u8> {
     type Error = WitnessStackError;
 
-    fn try_from(val: WitnessStack<F>) -> Result<Self, Self::Error> {
-        let buf = bincode::serialize(&val).unwrap();
+    fn try_from(val: &WitnessStack<F>) -> Result<Self, Self::Error> {
+        let buf = bincode::serialize(val).map_err(|e| WitnessStackError(e.into()))?;
         let mut deflater = GzEncoder::new(buf.as_slice(), Compression::best());
         let mut buf_c = Vec::new();
         deflater.read_to_end(&mut buf_c).map_err(|err| WitnessStackError(err.into()))?;
         Ok(buf_c)
     }
 }
 
+impl<F: Serialize> TryFrom<WitnessStack<F>> for Vec<u8> {
+    type Error = WitnessStackError;
+
+    fn try_from(val: WitnessStack<F>) -> Result<Self, Self::Error> {
+        Self::try_from(&val)
+    }
+}
+
 impl<F: for<'a> Deserialize<'a>> TryFrom<&[u8]> for WitnessStack<F> {
     type Error = WitnessStackError;
 
     fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
         let mut deflater = GzDecoder::new(bytes);
         let mut buf_d = Vec::new();
         deflater.read_to_end(&mut buf_d).map_err(|err| WitnessStackError(err.into()))?;
-        let witness_stack = bincode::deserialize(&buf_d).unwrap();
+        let witness_stack =
+            bincode::deserialize(&buf_d).map_err(|e| WitnessStackError(e.into()))?;
         Ok(witness_stack)
     }
 }

compiler/wasm/src/compile.rs

@@ -130,7 +130,7 @@ pub(crate) struct DependencyGraph {
     pub(crate) root_dependencies: Vec<CrateName>,
     pub(crate) library_dependencies: BTreeMap<CrateName, Vec<CrateName>>,
 }
-/// This is map contains the paths of all of the files in the entry-point crate and
+/// This map contains the paths of all of the files in the entry-point crate and
 /// the transitive dependencies of the entry-point crate.
 ///
 /// This is for all intents and purposes the file system that the compiler will use to resolve/compile

tooling/acvm_cli/src/cli/execute_cmd.rs

@@ -8,7 +8,7 @@ use bn254_blackbox_solver::Bn254BlackBoxSolver;
 use clap::Args;
 use nargo::PrintOutput;
 
-use nargo::{foreign_calls::DefaultForeignCallBuilder, ops::execute_program};
+use nargo::foreign_calls::DefaultForeignCallBuilder;
 use noir_artifact_cli::errors::CliError;
 use noir_artifact_cli::fs::artifact::read_bytecode_from_file;
 use noir_artifact_cli::fs::witness::save_witness_to_dir;
@@ -56,7 +56,7 @@ fn run_command(args: ExecuteCommand) -> Result<String, CliError> {
     )?;
     if args.output_witness.is_some() {
         save_witness_to_dir(
-            output_witness,
+            &output_witness,
             &args.output_witness.unwrap(),
             &args.working_directory,
         )?;
@@ -80,7 +80,8 @@ pub(crate) fn execute_program_from_witness(
 ) -> Result<WitnessStack<FieldElement>, CliError> {
     let program: Program<FieldElement> =
         Program::deserialize_program(bytecode).map_err(CliError::CircuitDeserializationError)?;
-    execute_program(
+
+    nargo::ops::execute_program(
         &program,
         inputs_map,
         &Bn254BlackBoxSolver(pedantic_solving),

tooling/artifact_cli/src/commands/execute_cmd.rs

@@ -1,18 +1,15 @@
-use std::{collections::BTreeMap, path::PathBuf};
+use std::path::PathBuf;
 
-use acir::{FieldElement, circuit::Program, native_types::WitnessStack};
 use bn254_blackbox_solver::Bn254BlackBoxSolver;
 use clap::Args;
-use color_eyre::eyre::{self, bail};
 
 use crate::{
     Artifact,
     errors::CliError,
-    fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir},
+    execution::{self, ExecutionResults},
 };
-use nargo::{NargoError, PrintOutput, foreign_calls::DefaultForeignCallBuilder};
-use noirc_abi::{Abi, input_parser::InputValue};
-use noirc_artifacts::debug::DebugArtifact;
+use nargo::{PrintOutput, foreign_calls::DefaultForeignCallBuilder};
+use noirc_driver::CompiledProgram;
 
 use super::parse_and_normalize_path;
 
@@ -21,106 +18,84 @@ use super::parse_and_normalize_path;
 pub struct ExecuteCommand {
     /// Path to the JSON build artifact (either a program or a contract).
     #[clap(long, short, value_parser = parse_and_normalize_path)]
-    artifact_path: PathBuf,
+    pub artifact_path: PathBuf,
 
     /// Path to the Prover.toml file which contains the inputs and the
     /// optional return value in ABI format.
     #[clap(long, short, value_parser = parse_and_normalize_path)]
-    prover_file: PathBuf,
+    pub prover_file: PathBuf,
 
     /// Path to the directory where the output witness should be saved.
     /// If empty then the results are discarded.
     #[clap(long, short, value_parser = parse_and_normalize_path)]
-    output_dir: Option<PathBuf>,
+    pub output_dir: Option<PathBuf>,
 
     /// Write the execution witness to named file
     ///
     /// Defaults to the name of the circuit being executed.
     #[clap(long, short)]
-    witness_name: Option<String>,
+    pub witness_name: Option<String>,
 
     /// Name of the function to execute, if the artifact is a contract.
     #[clap(long)]
-    contract_fn: Option<String>,
+    pub contract_fn: Option<String>,
 
     /// JSON RPC url to solve oracle calls.
     #[clap(long)]
-    oracle_resolver: Option<String>,
+    pub oracle_resolver: Option<String>,
 
     /// Use pedantic ACVM solving, i.e. double-check some black-box function assumptions when solving.
     #[clap(long, default_value_t = false)]
-    pedantic_solving: bool,
+    pub pedantic_solving: bool,
 }
 
-pub fn run(args: ExecuteCommand) -> eyre::Result<()> {
+pub fn run(args: ExecuteCommand) -> Result<(), CliError> {
     let artifact = Artifact::read_from_file(&args.artifact_path)?;
+    let artifact_name = args.artifact_path.file_stem().and_then(|s| s.to_str()).unwrap_or_default();
 
-    let circuit = match artifact {
-        Artifact::Program(program) => Circuit {
-            name: None,
-            abi: program.abi,
-            bytecode: program.bytecode,
-            debug_symbols: program.debug_symbols,
-            file_map: program.file_map,
-        },
+    let (circuit, circuit_name): (CompiledProgram, String) = match artifact {
+        Artifact::Program(program) => (program.into(), artifact_name.to_string()),
         Artifact::Contract(contract) => {
-            let names =
-                contract.functions.iter().map(|f| f.name.clone()).collect::<Vec<_>>().join(",");
+            let names = || contract.functions.iter().map(|f| f.name.clone()).collect::<Vec<_>>();
 
             let Some(ref name) = args.contract_fn else {
-                bail!("--contract-fn missing; options: [{names}]");
+                return Err(CliError::MissingContractFn { names: names() });
             };
-            let Some(function) = contract.functions.into_iter().find(|f| f.name == *name) else {
-                bail!("unknown --contract-fn '{name}'; options: [{names}]");
+            let Some(program) = contract.function_as_compiled_program(name) else {
+                return Err(CliError::UnknownContractFn { name: name.clone(), names: names() });
             };
 
-            Circuit {
-                name: Some(name.clone()),
-                abi: function.abi,
-                bytecode: function.bytecode,
-                debug_symbols: function.debug_symbols,
-                file_map: contract.file_map,
-            }
+            (program, format!("{artifact_name}::{name}"))
         }
     };
 
     match execute(&circuit, &args) {
-        Ok(solved) => {
-            save_witness(circuit, args, solved)?;
-        }
-        Err(CliError::CircuitExecutionError(err)) => {
-            show_diagnostic(circuit, err);
+        Ok(results) => {
+            execution::save_and_check_witness(
+                &circuit,
+                results,
+                &circuit_name,
+                args.output_dir.as_deref(),
+                args.witness_name.as_deref(),
+            )?;
         }
         Err(e) => {
-            bail!("failed to execute the circuit: {e}");
+            if let CliError::CircuitExecutionError(ref err) = e {
+                execution::show_diagnostic(&circuit, err);
+            }
+            // Still returning the error to facilitate command forwarding, to indicate that the command failed.
+            return Err(e);
         }
     }
     Ok(())
 }
 
-/// Parameters necessary to execute a circuit, display execution failures, etc.
-struct Circuit {
-    name: Option<String>,
-    abi: Abi,
-    bytecode: Program<FieldElement>,
-    debug_symbols: noirc_errors::debug_info::ProgramDebugInfo,
-    file_map: BTreeMap<fm::FileId, noirc_driver::DebugFile>,
-}
-
-struct SolvedWitnesses {
-    expected_return: Option<InputValue>,
-    actual_return: Option<InputValue>,
-    witness_stack: WitnessStack<FieldElement>,
-}
-
 /// Execute a circuit and return the output witnesses.
-fn execute(circuit: &Circuit, args: &ExecuteCommand) -> Result<SolvedWitnesses, CliError> {
-    let (input_map, expected_return) = read_inputs_from_file(&args.prover_file, &circuit.abi)?;
-
-    let initial_witness = circuit.abi.encode(&input_map, None)?;
-
+fn execute(circuit: &CompiledProgram, args: &ExecuteCommand) -> Result<ExecutionResults, CliError> {
     // TODO: Build a custom foreign call executor that reads from the Oracle transcript,
-    // and use it as a base for the default executor; see `DefaultForeignCallBuilder::build_with_base`
+    // and use it as a base for the default executor. Using it as the innermost rather
+    // than top layer so that any extra `print` added for debugging is handled by the
+    // default, rather than trying to match it to the transcript.
     let mut foreign_call_executor = DefaultForeignCallBuilder {
         output: PrintOutput::Stdout,
         enable_mocks: false,
@@ -130,80 +105,7 @@ fn execute(circuit: &Circuit, args: &ExecuteCommand) -> Result<SolvedWitnesses,
     }
     .build();
 
-    let witness_stack = nargo::ops::execute_program(
-        &circuit.bytecode,
-        initial_witness,
-        &Bn254BlackBoxSolver(args.pedantic_solving),
-        &mut foreign_call_executor,
-    )?;
-
-    let main_witness =
-        &witness_stack.peek().expect("Should have at least one witness on the stack").witness;
-
-    let (_, actual_return) = circuit.abi.decode(main_witness)?;
+    let blackbox_solver = Bn254BlackBoxSolver(args.pedantic_solving);
 
-    Ok(SolvedWitnesses { expected_return, actual_return, witness_stack })
-}
-
-/// Print an error stack trace, if possible.
-fn show_diagnostic(circuit: Circuit, err: NargoError<FieldElement>) {
-    if let Some(diagnostic) = nargo::errors::try_to_diagnose_runtime_error(
-        &err,
-        &circuit.abi,
-        &circuit.debug_symbols.debug_infos,
-    ) {
-        let debug_artifact = DebugArtifact {
-            debug_symbols: circuit.debug_symbols.debug_infos,
-            file_map: circuit.file_map,
-        };
-        diagnostic.report(&debug_artifact, false);
-    }
-}
-
-/// Print information about the witness and compare to expectations,
-/// returning errors if something isn't right.
-fn save_witness(
-    circuit: Circuit,
-    args: ExecuteCommand,
-    solved: SolvedWitnesses,
-) -> eyre::Result<()> {
-    let artifact = args.artifact_path.file_stem().and_then(|s| s.to_str()).unwrap_or_default();
-    let name = circuit
-        .name
-        .as_ref()
-        .map(|name| format!("{artifact}.{name}"))
-        .unwrap_or_else(|| artifact.to_string());
-
-    println!("[{}] Circuit witness successfully solved", name);
-
-    if let Some(ref witness_dir) = args.output_dir {
-        let witness_path = save_witness_to_dir(
-            solved.witness_stack,
-            &args.witness_name.unwrap_or_else(|| name.clone()),
-            witness_dir,
-        )?;
-        println!("[{}] Witness saved to {}", name, witness_path.display());
-    }
-
-    // Check that the circuit returned a non-empty result if the ABI expects a return value.
-    if let Some(ref expected) = circuit.abi.return_type {
-        if solved.actual_return.is_none() {
-            bail!("Missing return witness; expected a value of type {expected:?}");
-        }
-    }
-
-    // Check that if the prover file contained a `return` entry then that's what we got.
-    if let Some(expected) = solved.expected_return {
-        match solved.actual_return {
-            None => {
-                bail!("Missing return witness;\nexpected:\n{expected:?}");
-            }
-            Some(actual) if actual != expected => {
-                bail!("Unexpected return witness;\nexpected:\n{expected:?}\ngot:\n{actual:?}");
-            }
-            _ => {}
-        }
-    }
-
-    Ok(())
+    execution::execute(circuit, &blackbox_solver, &mut foreign_call_executor, &args.prover_file)
 }

tooling/artifact_cli/src/commands/mod.rs

@@ -1,3 +1,4 @@
+//! This module is for commands that we might want to invoke from `nargo` as-is.
 use std::path::PathBuf;
 
 use color_eyre::eyre;

tooling/artifact_cli/src/errors.rs

@@ -1,6 +1,10 @@
 use acir::FieldElement;
 use nargo::NargoError;
-use noirc_abi::errors::{AbiError, InputParserError};
+use noirc_abi::{
+    AbiReturnType,
+    errors::{AbiError, InputParserError},
+    input_parser::InputValue,
+};
 use std::path::PathBuf;
 use thiserror::Error;
 
@@ -61,4 +65,16 @@ pub enum CliError {
 
     #[error("Failed to serialize output witness: {0}")]
     OutputWitnessSerializationFailed(#[from] toml::ser::Error),
+
+    #[error("Unexpected return value: expected {expected:?}; got {actual:?}")]
+    UnexpectedReturn { expected: InputValue, actual: Option<InputValue> },
+
+    #[error("Missing return witnesses; expected {expected:?}")]
+    MissingReturn { expected: AbiReturnType },
+
+    #[error("Missing contract function name; options: {names:?}")]
+    MissingContractFn { names: Vec<String> },
+
+    #[error("Unknown contract function '{name}'; options: {names:?}")]
+    UnknownContractFn { name: String, names: Vec<String> },
 }