Skip to content

Commit

Permalink
feat: Add Acir debug information (#1864)
Browse files Browse the repository at this point in the history
* generates acir debug information

* reset location

* merge from master

* Display the location in  the error and use index from acvm

* update cargo.toml

* clean-up: restore unit tests

* restore the prover.toml

* notify unsatisfied constraint only once

* set the location and use it when inserting an instruction

* update to the corresponding acvm PR

* update cargo toml

* code review: remove temp code

* Code review

* Code review

* Update crates/noirc_errors/src/debug_info.rs

Co-authored-by: jfecher <jake@aztecprotocol.com>

* fix typo

* fix merge

* minor nit

* revert all test changes

* add noirc_errors as a dependency to nargo

* preprocess_program returns DebugInfo

* modify all methods which call preprocess_program

* change unwrap to expect

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>
  • Loading branch information
3 people authored Jul 12, 2023
1 parent 2bc7d25 commit 5ff8b53
Show file tree
Hide file tree
Showing 33 changed files with 354 additions and 107 deletions.
6 changes: 6 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ noir_wasm = { path = "crates/wasm" }

cfg-if = "1.0.0"
clap = { version = "4.1.4", features = ["derive"] }
codespan = "0.11.1"
codespan = {version = "0.11.1", features = ["serialization"]}
codespan-lsp = "0.11.1"
codespan-reporting = "0.11.1"
chumsky = { git = "https://github.com/jfecher/chumsky", rev = "ad9d312" }
Expand Down
1 change: 1 addition & 0 deletions crates/fm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ edition.workspace = true
codespan-reporting.workspace = true
cfg-if.workspace = true
rust-embed = "6.6.0"
serde.workspace = true

[target.'cfg(target_arch = "wasm32")'.dependencies]
wasm-bindgen.workspace = true
Expand Down
6 changes: 3 additions & 3 deletions crates/fm/src/file_map.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::FileManager;
use codespan_reporting::files::{SimpleFile, SimpleFiles};
use serde::{Deserialize, Serialize};
use std::path::PathBuf;

use crate::FileManager;

// XXX: File and FileMap serve as opaque types, so that the rest of the library does not need to import the dependency
// or worry about when we change the dep

Expand Down Expand Up @@ -34,7 +34,7 @@ impl From<&PathBuf> for PathString {
pub struct FileMap(SimpleFiles<PathString, String>);

// XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId
#[derive(Default, Debug, Clone, PartialEq, Eq, Copy, Hash)]
#[derive(Default, Debug, Clone, PartialEq, Eq, Copy, Hash, Serialize, Deserialize)]
pub struct FileId(usize);

impl FileId {
Expand Down
1 change: 1 addition & 0 deletions crates/nargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ iter-extended.workspace = true
toml.workspace = true
serde.workspace = true
thiserror.workspace = true
noirc_errors.workspace = true
20 changes: 12 additions & 8 deletions crates/nargo/src/ops/preprocess.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use acvm::ProofSystemCompiler;
use noirc_driver::{CompiledProgram, ContractFunction};
use noirc_errors::debug_info::DebugInfo;

use crate::artifacts::{contract::PreprocessedContractFunction, program::PreprocessedProgram};

Expand All @@ -11,7 +12,7 @@ pub fn preprocess_program<B: ProofSystemCompiler>(
include_keys: bool,
common_reference_string: &[u8],
compiled_program: CompiledProgram,
) -> Result<PreprocessedProgram, B::Error> {
) -> Result<(PreprocessedProgram, DebugInfo), B::Error> {
// TODO: currently `compiled_program`'s bytecode is already optimized for the backend.
// In future we'll need to apply those optimizations here.
let optimized_bytecode = compiled_program.circuit;
Expand All @@ -24,13 +25,16 @@ pub fn preprocess_program<B: ProofSystemCompiler>(
(None, None)
};

Ok(PreprocessedProgram {
backend: String::from(BACKEND_IDENTIFIER),
abi: compiled_program.abi,
bytecode: optimized_bytecode,
proving_key,
verification_key,
})
Ok((
PreprocessedProgram {
backend: String::from(BACKEND_IDENTIFIER),
abi: compiled_program.abi,
bytecode: optimized_bytecode,
proving_key,
verification_key,
},
compiled_program.debug,
))
}

pub fn preprocess_contract_function<B: ProofSystemCompiler>(
Expand Down
1 change: 1 addition & 0 deletions crates/nargo_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ noirc_frontend.workspace = true
noirc_abi.workspace = true
noirc_errors.workspace = true
acvm.workspace = true
fm.workspace = true
toml.workspace = true
serde.workspace = true
serde_json.workspace = true
Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/codegen_verifier_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ pub(crate) fn run<B: Backend>(
(common_reference_string, program)
}
None => {
let program =
let (program, _) =
compile_circuit(backend, config.program_dir.as_ref(), &args.compile_options)?;
let common_reference_string =
update_common_reference_string(backend, &common_reference_string, &program.circuit)
.map_err(CliError::CommonReferenceStringError)?;
let program = preprocess_program(backend, true, &common_reference_string, program)
let (program, _) = preprocess_program(backend, true, &common_reference_string, program)
.map_err(CliError::ProofSystemCompilerError)?;
(common_reference_string, program)
}
Expand Down
42 changes: 26 additions & 16 deletions crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use acvm::acir::circuit::OpcodeLabel;
use acvm::{acir::circuit::Circuit, Backend};
use iter_extended::try_vecmap;
use iter_extended::vecmap;
use nargo::{artifacts::contract::PreprocessedContract, NargoError};
use noirc_driver::{
compile_contracts, compile_main, CompileOptions, CompiledProgram, ErrorsAndWarnings, Warnings,
Expand Down Expand Up @@ -68,8 +70,7 @@ pub(crate) fn run<B: Backend>(
try_vecmap(contracts, |contract| {
let preprocessed_contract_functions =
try_vecmap(contract.functions, |mut func| {
func.bytecode = optimize_circuit(backend, func.bytecode)?;

func.bytecode = optimize_circuit(backend, func.bytecode)?.0;
common_reference_string = update_common_reference_string(
backend,
&common_reference_string,
Expand Down Expand Up @@ -100,13 +101,12 @@ pub(crate) fn run<B: Backend>(
);
}
} else {
let program = compile_circuit(backend, &config.program_dir, &args.compile_options)?;

let (program, _) = compile_circuit(backend, &config.program_dir, &args.compile_options)?;
common_reference_string =
update_common_reference_string(backend, &common_reference_string, &program.circuit)
.map_err(CliError::CommonReferenceStringError)?;

let preprocessed_program =
let (preprocessed_program, _) =
preprocess_program(backend, args.include_keys, &common_reference_string, program)
.map_err(CliError::ProofSystemCompilerError)?;
save_program_to_file(&preprocessed_program, &args.circuit_name, circuit_dir);
Expand All @@ -121,27 +121,37 @@ pub(crate) fn compile_circuit<B: Backend>(
backend: &B,
program_dir: &Path,
compile_options: &CompileOptions,
) -> Result<CompiledProgram, CliError<B>> {
) -> Result<(CompiledProgram, Context), CliError<B>> {
let mut context = resolve_root_manifest(program_dir)?;
let result = compile_main(&mut context, compile_options);
let mut program = report_errors(result, &context, compile_options.deny_warnings)?;

// Apply backend specific optimizations.
program.circuit = optimize_circuit(backend, program.circuit).unwrap();
Ok(program)
let (optimized_circuit, opcode_labels) = optimize_circuit(backend, program.circuit)
.expect("Backend does not support an opcode that is in the IR");

program.circuit = optimized_circuit;
let opcode_ids = vecmap(opcode_labels, |label| match label {
OpcodeLabel::Unresolved => {
unreachable!("Compiled circuit opcodes must resolve to some index")
}
OpcodeLabel::Resolved(index) => index as usize,
});
program.debug.update_acir(opcode_ids);

Ok((program, context))
}

pub(super) fn optimize_circuit<B: Backend>(
backend: &B,
circuit: Circuit,
) -> Result<Circuit, CliError<B>> {
let (optimized_circuit, _): (Circuit, Vec<acvm::acir::circuit::OpcodeLabel>) =
acvm::compiler::compile(circuit, backend.np_language(), |opcode| {
backend.supports_opcode(opcode)
})
.map_err(|_| NargoError::CompilationError)?;

Ok(optimized_circuit)
) -> Result<(Circuit, Vec<OpcodeLabel>), CliError<B>> {
let result = acvm::compiler::compile(circuit, backend.np_language(), |opcode| {
backend.supports_opcode(opcode)
})
.map_err(|_| NargoError::CompilationError)?;

Ok(result)
}

/// Helper function for reporting any errors in a Result<(T, Warnings), ErrorsAndWarnings>
Expand Down
67 changes: 60 additions & 7 deletions crates/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
use std::path::Path;

use acvm::acir::circuit::OpcodeLabel;
use acvm::acir::{circuit::Circuit, native_types::WitnessMap};
use acvm::Backend;
use clap::Args;
use nargo::NargoError;
use noirc_abi::input_parser::{Format, InputValue};
use noirc_abi::{Abi, InputMap};
use noirc_driver::{CompileOptions, CompiledProgram};
use noirc_errors::{debug_info::DebugInfo, CustomDiagnostic};
use noirc_frontend::hir::Context;

use super::fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir};
use super::NargoConfig;
Expand Down Expand Up @@ -57,29 +61,78 @@ fn execute_with_path<B: Backend>(
prover_name: String,
compile_options: &CompileOptions,
) -> Result<(Option<InputValue>, WitnessMap), CliError<B>> {
let CompiledProgram { abi, circuit } = compile_circuit(backend, program_dir, compile_options)?;
let (compiled_program, context) = compile_circuit(backend, program_dir, compile_options)?;
let CompiledProgram { abi, circuit, debug } = compiled_program;

// Parse the initial witness values from Prover.toml
let (inputs_map, _) =
read_inputs_from_file(program_dir, prover_name.as_str(), Format::Toml, &abi)?;

let solved_witness = execute_program(backend, circuit, &abi, &inputs_map)?;

let solved_witness =
execute_program(backend, circuit, &abi, &inputs_map, Some((debug, context)))?;
let public_abi = abi.public_abi();
let (_, return_value) = public_abi.decode(&solved_witness)?;

Ok((return_value, solved_witness))
}

fn extract_unsatisfied_constraint_from_nargo_error(nargo_err: &NargoError) -> Option<usize> {
let solving_err = match nargo_err {
nargo::NargoError::SolvingError(err) => err,
_ => return None,
};

match solving_err {
acvm::pwg::OpcodeResolutionError::UnsatisfiedConstrain { opcode_label } => {
match opcode_label {
OpcodeLabel::Unresolved => {
unreachable!("Cannot resolve index for unsatisfied constraint")
}
OpcodeLabel::Resolved(opcode_index) => Some(*opcode_index as usize),
}
}
_ => None,
}
}
fn report_unsatisfied_constraint_error(
opcode_idx: Option<usize>,
debug: &DebugInfo,
context: &Context,
) {
if let Some(opcode_index) = opcode_idx {
if let Some(loc) = debug.opcode_location(opcode_index) {
noirc_errors::reporter::report(
&context.file_manager,
&CustomDiagnostic::simple_error(
"Unsatisfied constraint".to_string(),
"Constraint failed".to_string(),
loc.span,
),
Some(loc.file),
false,
);
}
}
}

pub(crate) fn execute_program<B: Backend>(
backend: &B,
circuit: Circuit,
abi: &Abi,
inputs_map: &InputMap,
debug_data: Option<(DebugInfo, Context)>,
) -> Result<WitnessMap, CliError<B>> {
let initial_witness = abi.encode(inputs_map, None)?;

let solved_witness = nargo::ops::execute_circuit(backend, circuit, initial_witness)?;

Ok(solved_witness)
let solved_witness_err = nargo::ops::execute_circuit(backend, circuit, initial_witness);
match solved_witness_err {
Ok(solved_witness) => Ok(solved_witness),
Err(err) => {
if let Some((debug, context)) = debug_data {
let opcode_idx = extract_unsatisfied_constraint_from_nargo_error(&err);
report_unsatisfied_constraint_error(opcode_idx, &debug, &context);
}

Err(crate::errors::CliError::NargoError(err))
}
}
}
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/gates_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn count_gates_with_path<B: Backend, P: AsRef<Path>>(
program_dir: P,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let compiled_program = compile_circuit(backend, program_dir.as_ref(), compile_options)?;
let (compiled_program, _) = compile_circuit(backend, program_dir.as_ref(), compile_options)?;
let num_opcodes = compiled_program.circuit.opcodes.len();

println!(
Expand Down
16 changes: 9 additions & 7 deletions crates/nargo_cli/src/cli/prove_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub(crate) fn prove_with_path<B: Backend, P: AsRef<Path>>(
) -> Result<Option<PathBuf>, CliError<B>> {
let common_reference_string = read_cached_common_reference_string();

let (common_reference_string, preprocessed_program) = match circuit_build_path {
let (common_reference_string, preprocessed_program, debug_data) = match circuit_build_path {
Some(circuit_build_path) => {
let program = read_program_from_file(circuit_build_path)?;
let common_reference_string = update_common_reference_string(
Expand All @@ -100,16 +100,18 @@ pub(crate) fn prove_with_path<B: Backend, P: AsRef<Path>>(
&program.bytecode,
)
.map_err(CliError::CommonReferenceStringError)?;
(common_reference_string, program)
(common_reference_string, program, None)
}
None => {
let program = compile_circuit(backend, program_dir.as_ref(), compile_options)?;
let (program, context) =
compile_circuit(backend, program_dir.as_ref(), compile_options)?;
let common_reference_string =
update_common_reference_string(backend, &common_reference_string, &program.circuit)
.map_err(CliError::CommonReferenceStringError)?;
let program = preprocess_program(backend, true, &common_reference_string, program)
.map_err(CliError::ProofSystemCompilerError)?;
(common_reference_string, program)
let (program, debug) =
preprocess_program(backend, true, &common_reference_string, program)
.map_err(CliError::ProofSystemCompilerError)?;
(common_reference_string, program, Some((debug, context)))
}
};

Expand All @@ -122,7 +124,7 @@ pub(crate) fn prove_with_path<B: Backend, P: AsRef<Path>>(
let (inputs_map, _) =
read_inputs_from_file(&program_dir, prover_name.as_str(), Format::Toml, &abi)?;

let solved_witness = execute_program(backend, bytecode.clone(), &abi, &inputs_map)?;
let solved_witness = execute_program(backend, bytecode.clone(), &abi, &inputs_map, debug_data)?;

// Write public inputs into Verifier.toml
let public_abi = abi.public_abi();
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ fn run_test<B: Backend>(
let mut program = compile_no_check(context, config, main)
.map_err(|_| CliError::Generic(format!("Test '{test_name}' failed to compile")))?;
// Note: We could perform this test using the unoptimized ACIR as generated by `compile_no_check`.
program.circuit = optimize_circuit(backend, program.circuit).unwrap();
program.circuit = optimize_circuit(backend, program.circuit).unwrap().0;

// Run the backend to ensure the PWG evaluates functions like std::hash::pedersen,
// otherwise constraints involving these expressions will not error.
Expand Down
Loading

0 comments on commit 5ff8b53

Please sign in to comment.