Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add Acir debug information #1864

Merged
merged 29 commits into from
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
b749264
generates acir debug information
guipublic Jul 5, 2023
7745e8c
reset location
guipublic Jul 5, 2023
c4a5912
Merge branch 'master' into gd/debug_location
guipublic Jul 6, 2023
f2d847c
merge from master
guipublic Jul 6, 2023
9e65375
Display the location in the error and use index from acvm
guipublic Jul 6, 2023
f338a72
update cargo.toml
guipublic Jul 6, 2023
3bf33a5
clean-up: restore unit tests
guipublic Jul 6, 2023
f52d26e
restore the prover.toml
guipublic Jul 6, 2023
e72fd88
Merge branch 'master' into gd/debug_location
guipublic Jul 7, 2023
20a34bf
notify unsatisfied constraint only once
guipublic Jul 7, 2023
b863806
set the location and use it when inserting an instruction
guipublic Jul 7, 2023
bf27d1a
Merge remote-tracking branch 'origin/master' into gd/debug_location
kevaundray Jul 10, 2023
0ee7a8f
update to the corresponding acvm PR
guipublic Jul 11, 2023
95992e4
Merge branch 'master' into gd/debug_location
guipublic Jul 11, 2023
1ffb258
update cargo toml
guipublic Jul 11, 2023
0ceb640
Merge branch 'master' into gd/debug_location
guipublic Jul 11, 2023
90afcae
code review: remove temp code
guipublic Jul 12, 2023
7e48183
Code review
guipublic Jul 12, 2023
8bd17d9
Code review
guipublic Jul 12, 2023
e066dea
Update crates/noirc_errors/src/debug_info.rs
kevaundray Jul 12, 2023
0d21f7e
Merge remote-tracking branch 'origin/master' into gd/debug_location
kevaundray Jul 12, 2023
d440022
fix typo
kevaundray Jul 12, 2023
2ce27f7
fix merge
kevaundray Jul 12, 2023
97b3f8d
minor nit
kevaundray Jul 12, 2023
f1587b8
revert all test changes
kevaundray Jul 12, 2023
566648d
add noirc_errors as a dependency to nargo
kevaundray Jul 12, 2023
82241b6
preprocess_program returns DebugInfo
kevaundray Jul 12, 2023
aa7a8ea
modify all methods which call preprocess_program
kevaundray Jul 12, 2023
b0c34b6
change unwrap to expect
kevaundray Jul 12, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
guipublic marked this conversation as resolved.
Show resolved Hide resolved
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