From 4c3ab1ab6b404ecfeb896ef4f65f93233948fb46 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 11 Jun 2025 15:46:52 -0300 Subject: [PATCH 1/6] chore: run `nargo interpret` against all execution_success programs --- tooling/nargo_cli/build.rs | 88 ++++++++++++++++++++++++++++++ tooling/nargo_cli/tests/execute.rs | 4 ++ 2 files changed, 92 insertions(+) diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index 0471ba722e0..bba52332877 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -42,6 +42,8 @@ fn main() { generate_compile_success_with_bug_tests(&mut test_file, &test_dir); generate_compile_failure_tests(&mut test_file, &test_dir); + generate_interpret_execution_success_tests(&mut test_file, &test_dir); + generate_fuzzing_failure_tests(&mut test_file, &test_dir); generate_nargo_expand_execution_success_tests(&mut test_file, &test_dir); @@ -115,6 +117,56 @@ const TESTS_WITH_EXPECTED_WARNINGS: [&str; 5] = [ "brillig_continue_break", ]; +/// `nargo interpret` ignored tests, either because they don't currently work or +/// becuase they are too slow to run. +const IGNORED_INTERPRET_EXECUTION_TESTS: [&str; 43] = [ + // slow + "regression_4709", + // bug + "array_dynamic_nested_blackbox_input", + "array_oob_regression_7965", + "array_oob_regression_7975", + "as_witness", + "brillig_block_parameter_liveness", + "brillig_cow", + "brillig_cow_regression", + "databus", + "databus_composite_calldata", + "databus_two_calldata", + "databus_two_calldata_simple", + "fold_numeric_generic_poseidon", + "global_array_rc_regression_8259", + "hash_to_field", + "hashmap", + "inline_decompose_hint_brillig_call", + "multi_scalar_mul", + "nested_array_dynamic", + "nested_if_then_block_same_cond", + "no_predicates_numeric_generic_poseidon", + "pedersen_commitment", + "ram_blowup_regression", + "regression_11294", + "regression_3889", + "regression_4088", + "regression_5252", + "regression_7128", + "regression_7612", + "regression_7744", + "regression_8174", + "regression_struct_array_conditional", + "signed_div", + "simple_shield", + "slice_loop", + "struct_array_inputs", + "struct_fields_ordering", + "struct_inputs", + "to_be_bytes", + "to_le_bytes", + "tuple_inputs", + "uhashmap", + "unrolling_regression_8333", +]; + /// These tests are ignored because making them work involves a more complex test code that /// might not be worth it. /// Others are ignored because of existing bugs in `nargo expand`. @@ -381,6 +433,7 @@ fn test_{test_name}() {{ ) .expect("Could not write templated test file."); } + fn generate_execution_success_tests(test_file: &mut File, test_data_dir: &Path) { let test_type = "execution_success"; let test_cases = read_test_cases(test_data_dir, test_type); @@ -699,6 +752,41 @@ fn generate_compile_failure_tests(test_file: &mut File, test_data_dir: &Path) { writeln!(test_file, "}}").unwrap(); } +fn generate_interpret_execution_success_tests(test_file: &mut File, test_data_dir: &Path) { + let test_type = "execution_success"; + let test_cases = read_test_cases(test_data_dir, test_type); + + writeln!( + test_file, + "mod interpret_{test_type} {{ + use super::*; + " + ) + .unwrap(); + for (test_name, test_dir) in test_cases { + if IGNORED_INTERPRET_EXECUTION_TESTS.contains(&test_name.as_str()) { + continue; + } + + let test_dir = test_dir.display(); + + generate_test_cases( + test_file, + &test_name, + &test_dir, + "interpret", + "interpret_execution_success(nargo);", + &MatrixConfig { + vary_brillig: !IGNORED_BRILLIG_TESTS.contains(&test_name.as_str()), + vary_inliner: true, + min_inliner: min_inliner(&test_name), + max_inliner: max_inliner(&test_name), + }, + ); + } + writeln!(test_file, "}}").unwrap(); +} + /// Here we check, for every program in `test_programs/exeuction_success`, that: /// 1. `nargo expand` works on it /// 2. That the output of the original program is the same as the output of the expanded program diff --git a/tooling/nargo_cli/tests/execute.rs b/tooling/nargo_cli/tests/execute.rs index 922fcb49433..7fb78ef3245 100644 --- a/tooling/nargo_cli/tests/execute.rs +++ b/tooling/nargo_cli/tests/execute.rs @@ -322,6 +322,10 @@ mod tests { }) } + fn interpret_execution_success(mut nargo: Command) { + nargo.assert().success(); + } + fn nargo_expand_execute(test_program_dir: PathBuf) { // First run `nargo execute` on the original code to get the output let mut nargo = Command::cargo_bin("nargo").unwrap(); From 458c7eb620e302315151e5677e6d266733621060 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 11 Jun 2025 16:15:14 -0300 Subject: [PATCH 2/6] Improve `AbiTypeMismatch` error message --- tooling/noirc_abi/src/errors.rs | 4 ++-- tooling/noirc_abi/src/input_parser/json.rs | 14 ++++++++++++-- tooling/noirc_abi/src/input_parser/toml.rs | 14 ++++++++++++-- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/tooling/noirc_abi/src/errors.rs b/tooling/noirc_abi/src/errors.rs index f08f7b21721..2d5799ccd0e 100644 --- a/tooling/noirc_abi/src/errors.rs +++ b/tooling/noirc_abi/src/errors.rs @@ -26,8 +26,8 @@ pub enum InputParserError { FieldElement::modulus() )] InputExceedsFieldModulus { arg_name: String, value: String }, - #[error("cannot parse value into {0:?}")] - AbiTypeMismatch(AbiType), + #[error("cannot parse value {0} into {1:?}")] + AbiTypeMismatch(String, AbiType), #[error("Expected argument `{0}`, but none was found")] MissingArgument(String), } diff --git a/tooling/noirc_abi/src/input_parser/json.rs b/tooling/noirc_abi/src/input_parser/json.rs index a7285eae909..7391c5d62a3 100644 --- a/tooling/noirc_abi/src/input_parser/json.rs +++ b/tooling/noirc_abi/src/input_parser/json.rs @@ -120,7 +120,12 @@ impl JsonTypes { JsonTypes::Array(fields) } - _ => return Err(InputParserError::AbiTypeMismatch(abi_type.clone())), + _ => { + return Err(InputParserError::AbiTypeMismatch( + format!("{value:?}"), + abi_type.clone(), + )); + } }; Ok(json_value) } @@ -212,7 +217,12 @@ impl InputValue { InputValue::Vec(tuple_fields) } - (_, _) => return Err(InputParserError::AbiTypeMismatch(param_type.clone())), + (value, _) => { + return Err(InputParserError::AbiTypeMismatch( + format!("{value:?}"), + param_type.clone(), + )); + } }; Ok(input_value) diff --git a/tooling/noirc_abi/src/input_parser/toml.rs b/tooling/noirc_abi/src/input_parser/toml.rs index 2e2d12f853c..0314fe3de6c 100644 --- a/tooling/noirc_abi/src/input_parser/toml.rs +++ b/tooling/noirc_abi/src/input_parser/toml.rs @@ -118,7 +118,12 @@ impl TomlTypes { TomlTypes::Array(fields) } - _ => return Err(InputParserError::AbiTypeMismatch(abi_type.clone())), + _ => { + return Err(InputParserError::AbiTypeMismatch( + format!("{value:?}"), + abi_type.clone(), + )); + } }; Ok(toml_value) } @@ -197,7 +202,12 @@ impl InputValue { InputValue::Vec(tuple_fields) } - (_, _) => return Err(InputParserError::AbiTypeMismatch(param_type.clone())), + (value, _) => { + return Err(InputParserError::AbiTypeMismatch( + format!("{value:?}"), + param_type.clone(), + )); + } }; Ok(input_value) From d252295001c101939eeeb185193ba9c8e844ee78 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 11 Jun 2025 16:38:31 -0300 Subject: [PATCH 3/6] fix: build ABI from program and context --- compiler/noirc_driver/src/abi_gen.rs | 2 +- compiler/noirc_driver/src/lib.rs | 3 ++- tooling/nargo_cli/build.rs | 10 +--------- tooling/nargo_cli/src/cli/interpret_cmd.rs | 15 ++++++++++----- 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/compiler/noirc_driver/src/abi_gen.rs b/compiler/noirc_driver/src/abi_gen.rs index 984659a9dd6..12f5709dbd2 100644 --- a/compiler/noirc_driver/src/abi_gen.rs +++ b/compiler/noirc_driver/src/abi_gen.rs @@ -23,7 +23,7 @@ use noirc_frontend::{ /// Arranges a function signature and a generated circuit's return witnesses into a /// `noirc_abi::Abi`. -pub(super) fn gen_abi( +pub fn gen_abi( context: &Context, func_id: &FuncId, return_visibility: Visibility, diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index d6e7d366738..f1f7f0ab419 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -38,6 +38,7 @@ mod stdlib; use debug::filter_relevant_files; +pub use abi_gen::gen_abi; pub use contract::{CompiledContract, CompiledContractOutputs, ContractFunction}; pub use debug::DebugFile; pub use noirc_frontend::graph::{CrateId, CrateName}; @@ -793,7 +794,7 @@ pub fn compile_no_check( create_program(program, &ssa_evaluator_options)? }; - let abi = abi_gen::gen_abi(context, &main_function, return_visibility, error_types); + let abi = gen_abi(context, &main_function, return_visibility, error_types); let file_map = filter_relevant_files(&debug, &context.file_manager); Ok(CompiledProgram { diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index bba52332877..84a2ef77271 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -119,7 +119,7 @@ const TESTS_WITH_EXPECTED_WARNINGS: [&str; 5] = [ /// `nargo interpret` ignored tests, either because they don't currently work or /// becuase they are too slow to run. -const IGNORED_INTERPRET_EXECUTION_TESTS: [&str; 43] = [ +const IGNORED_INTERPRET_EXECUTION_TESTS: [&str; 35] = [ // slow "regression_4709", // bug @@ -128,7 +128,6 @@ const IGNORED_INTERPRET_EXECUTION_TESTS: [&str; 43] = [ "array_oob_regression_7975", "as_witness", "brillig_block_parameter_liveness", - "brillig_cow", "brillig_cow_regression", "databus", "databus_composite_calldata", @@ -137,33 +136,26 @@ const IGNORED_INTERPRET_EXECUTION_TESTS: [&str; 43] = [ "fold_numeric_generic_poseidon", "global_array_rc_regression_8259", "hash_to_field", - "hashmap", "inline_decompose_hint_brillig_call", "multi_scalar_mul", - "nested_array_dynamic", "nested_if_then_block_same_cond", "no_predicates_numeric_generic_poseidon", - "pedersen_commitment", "ram_blowup_regression", "regression_11294", "regression_3889", - "regression_4088", "regression_5252", "regression_7128", "regression_7612", "regression_7744", "regression_8174", "regression_struct_array_conditional", - "signed_div", "simple_shield", "slice_loop", "struct_array_inputs", - "struct_fields_ordering", "struct_inputs", "to_be_bytes", "to_le_bytes", "tuple_inputs", - "uhashmap", "unrolling_regression_8333", ]; diff --git a/tooling/nargo_cli/src/cli/interpret_cmd.rs b/tooling/nargo_cli/src/cli/interpret_cmd.rs index 46e31758de2..2701b2915c8 100644 --- a/tooling/nargo_cli/src/cli/interpret_cmd.rs +++ b/tooling/nargo_cli/src/cli/interpret_cmd.rs @@ -1,5 +1,7 @@ //! Use the SSA Interpreter to execute a SSA after a certain pass. +use std::collections::BTreeMap; + use acvm::acir::circuit::ExpressionWidth; use fm::{FileId, FileManager}; use nargo::constants::PROVER_INPUT_FILE; @@ -7,7 +9,8 @@ use nargo::ops::report_errors; use nargo::package::Package; use nargo::workspace::Workspace; use nargo_toml::PackageSelection; -use noirc_driver::{CompilationResult, CompileOptions}; +use noirc_abi::Abi; +use noirc_driver::{CompilationResult, CompileOptions, gen_abi}; use clap::Args; use noirc_errors::CustomDiagnostic; @@ -80,7 +83,7 @@ pub(crate) fn run(args: InterpretCommand, workspace: Workspace) -> Result<(), Cl ); // Report warnings and get the AST, or exit if the compilation failed. - let program = report_errors( + let (program, abi) = report_errors( program_result, &file_manager, args.compile_options.deny_warnings, @@ -88,7 +91,6 @@ pub(crate) fn run(args: InterpretCommand, workspace: Workspace) -> Result<(), Cl )?; // Parse the inputs and convert them to what the SSA interpreter expects. - let abi = noir_ast_fuzzer::program_abi(&program); let prover_file = package.root_dir.join(&args.prover_name).with_extension("toml"); let (prover_input, return_value) = noir_artifact_cli::fs::inputs::read_inputs_from_file(&prover_file, &abi)?; @@ -154,7 +156,7 @@ fn compile_into_program( workspace: &Workspace, package: &Package, options: &CompileOptions, -) -> CompilationResult { +) -> CompilationResult<(Program, Abi)> { let (mut context, crate_id) = nargo::prepare_package(file_manager, parsed_files, package); if options.disable_comptime_printing { context.disable_comptime_printing(); @@ -191,7 +193,10 @@ fn compile_into_program( println!("{program}"); } - Ok((program, warnings)) + let error_types = BTreeMap::default(); + let abi = gen_abi(&context, &main_id, program.return_visibility(), error_types); + + Ok(((program, abi), warnings)) } fn to_ssa_options(options: &CompileOptions) -> SsaEvaluatorOptions { From 093254886adba5d12d9e0d4aceceaefea6771250 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 11 Jun 2025 16:48:46 -0300 Subject: [PATCH 4/6] Explain failures --- tooling/nargo_cli/build.rs | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index 84a2ef77271..00f57a4eea2 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -122,40 +122,73 @@ const TESTS_WITH_EXPECTED_WARNINGS: [&str; 5] = [ const IGNORED_INTERPRET_EXECUTION_TESTS: [&str; 35] = [ // slow "regression_4709", - // bug + // panic: index out of bounds "array_dynamic_nested_blackbox_input", + // gives a different result with `--force-brillig` "array_oob_regression_7965", + // gives a different result with `--force-brillig` "array_oob_regression_7975", + // panic: FunctionReturnedIncorrectArgCount "as_witness", + // wrong result "brillig_block_parameter_liveness", + // panic: BlockArgumentCountMismatch "brillig_cow_regression", + // wrong result "databus", + // panic: index out of bounds "databus_composite_calldata", + // wrong result "databus_two_calldata", + // wrong result "databus_two_calldata_simple", + // panic: IntrinsicArgumentCountMismatch "fold_numeric_generic_poseidon", + // gives a different result with `--force-brillig` "global_array_rc_regression_8259", + // panic: IntrinsicArgumentCountMismatch "hash_to_field", + // panic: Internal(TypeError) "inline_decompose_hint_brillig_call", + // panic: Internal(TypeError) "multi_scalar_mul", + // gives a different result with `--force-brillig` "nested_if_then_block_same_cond", + // panic: IntrinsicArgumentCountMismatch "no_predicates_numeric_generic_poseidon", + // panic: IntrinsicArgumentCountMismatch "ram_blowup_regression", + // panic: index out of bounds "regression_11294", + // panic: Internal(TypeError) "regression_3889", + // panic: IntrinsicArgumentCountMismatch "regression_5252", + // panic: IntrinsicArgumentCountMismatch "regression_7128", + // panic: index out of bounds "regression_7612", + // gives a wrong result "regression_7744", + // gives a wrong result "regression_8174", + // panic: index out of bounds "regression_struct_array_conditional", + // panic Internal(TypeError) "simple_shield", + // panic: index out of bounds "slice_loop", + // panic: index out of bounds "struct_array_inputs", + // panic: BlockArgumentCountMismatch "struct_inputs", + // panic: IntrinsicArgumentCountMismatch "to_be_bytes", + // panic: IntrinsicArgumentCountMismatch "to_le_bytes", + // panic: BlockArgumentCountMismatch "tuple_inputs", + // panic: IntrinsicArgumentCountMismatch "unrolling_regression_8333", ]; From d126da54953bec5c77f7cbe587c3262a2e5d145c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 11 Jun 2025 16:57:40 -0300 Subject: [PATCH 5/6] Fix tests and improve error message --- tooling/noirc_abi/src/errors.rs | 2 +- tooling/noirc_abi_wasm/test/browser/errors.test.ts | 4 ++-- tooling/noirc_abi_wasm/test/node/errors.test.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tooling/noirc_abi/src/errors.rs b/tooling/noirc_abi/src/errors.rs index 2d5799ccd0e..06ab58bc7d7 100644 --- a/tooling/noirc_abi/src/errors.rs +++ b/tooling/noirc_abi/src/errors.rs @@ -26,7 +26,7 @@ pub enum InputParserError { FieldElement::modulus() )] InputExceedsFieldModulus { arg_name: String, value: String }, - #[error("cannot parse value {0} into {1:?}")] + #[error("cannot parse value `{0}` into {1:?}")] AbiTypeMismatch(String, AbiType), #[error("Expected argument `{0}`, but none was found")] MissingArgument(String), diff --git a/tooling/noirc_abi_wasm/test/browser/errors.test.ts b/tooling/noirc_abi_wasm/test/browser/errors.test.ts index cc060cff4d6..7f3fc53fcff 100644 --- a/tooling/noirc_abi_wasm/test/browser/errors.test.ts +++ b/tooling/noirc_abi_wasm/test/browser/errors.test.ts @@ -16,11 +16,11 @@ it('errors when an integer input overflows', async () => { it('errors when passing a field in place of an array', async () => { const { abi, inputs } = await import('../shared/field_as_array'); - expect(() => abiEncode(abi, inputs)).to.throw('cannot parse value into Array { length: 2, typ: Field }'); + expect(() => abiEncode(abi, inputs)).to.throw('cannot parse value `String("1")` into Array { length: 2, typ: Field }'); }); it('errors when passing an array in place of a field', async () => { const { abi, inputs } = await import('../shared/array_as_field'); - expect(() => abiEncode(abi, inputs)).to.throw('cannot parse value into Field'); + expect(() => abiEncode(abi, inputs)).to.throw('cannot parse value `Array([String("1"), String("2")])` into Field'); }); diff --git a/tooling/noirc_abi_wasm/test/node/errors.test.ts b/tooling/noirc_abi_wasm/test/node/errors.test.ts index 491fd5a5671..03cfdea99ce 100644 --- a/tooling/noirc_abi_wasm/test/node/errors.test.ts +++ b/tooling/noirc_abi_wasm/test/node/errors.test.ts @@ -12,11 +12,11 @@ it('errors when an integer input overflows', async () => { it('errors when passing a field in place of an array', async () => { const { abi, inputs } = await import('../shared/field_as_array'); - expect(() => abiEncode(abi, inputs)).to.throw('cannot parse value into Array { length: 2, typ: Field }'); + expect(() => abiEncode(abi, inputs)).to.throw('cannot parse value `String("1")` into Array { length: 2, typ: Field }'); }); it('errors when passing an array in place of a field', async () => { const { abi, inputs } = await import('../shared/array_as_field'); - expect(() => abiEncode(abi, inputs)).to.throw('cannot parse value into Field'); + expect(() => abiEncode(abi, inputs)).to.throw('cannot parse value `Array([String("1"), String("2")])` into Field'); }); From db7dafab805d4c74ad81605398ae06e6f4070524 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 11 Jun 2025 17:18:08 -0300 Subject: [PATCH 6/6] yarn lint --- tooling/noirc_abi_wasm/test/browser/errors.test.ts | 4 +++- tooling/noirc_abi_wasm/test/node/errors.test.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tooling/noirc_abi_wasm/test/browser/errors.test.ts b/tooling/noirc_abi_wasm/test/browser/errors.test.ts index 7f3fc53fcff..821d0c01e0b 100644 --- a/tooling/noirc_abi_wasm/test/browser/errors.test.ts +++ b/tooling/noirc_abi_wasm/test/browser/errors.test.ts @@ -16,7 +16,9 @@ it('errors when an integer input overflows', async () => { it('errors when passing a field in place of an array', async () => { const { abi, inputs } = await import('../shared/field_as_array'); - expect(() => abiEncode(abi, inputs)).to.throw('cannot parse value `String("1")` into Array { length: 2, typ: Field }'); + expect(() => abiEncode(abi, inputs)).to.throw( + 'cannot parse value `String("1")` into Array { length: 2, typ: Field }', + ); }); it('errors when passing an array in place of a field', async () => { diff --git a/tooling/noirc_abi_wasm/test/node/errors.test.ts b/tooling/noirc_abi_wasm/test/node/errors.test.ts index 03cfdea99ce..db9c8ca44c6 100644 --- a/tooling/noirc_abi_wasm/test/node/errors.test.ts +++ b/tooling/noirc_abi_wasm/test/node/errors.test.ts @@ -12,7 +12,9 @@ it('errors when an integer input overflows', async () => { it('errors when passing a field in place of an array', async () => { const { abi, inputs } = await import('../shared/field_as_array'); - expect(() => abiEncode(abi, inputs)).to.throw('cannot parse value `String("1")` into Array { length: 2, typ: Field }'); + expect(() => abiEncode(abi, inputs)).to.throw( + 'cannot parse value `String("1")` into Array { length: 2, typ: Field }', + ); }); it('errors when passing an array in place of a field', async () => {