From 54fc751c134114d029ec76a66639cee0e8f02e96 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Thu, 19 Dec 2024 09:20:22 +0000 Subject: [PATCH] feat!: `nargo::ops::test::run_test` generic in `ForeignCallExecutor` (#6858) --- tooling/lsp/src/requests/test_run.rs | 13 +++- tooling/nargo/Cargo.toml | 11 ++-- tooling/nargo/src/foreign_calls/default.rs | 59 ++++++++++++++++++ tooling/nargo/src/foreign_calls/mocker.rs | 3 +- tooling/nargo/src/foreign_calls/mod.rs | 66 ++++---------------- tooling/nargo/src/lib.rs | 5 +- tooling/nargo/src/ops/mod.rs | 9 +-- tooling/nargo/src/ops/test.rs | 70 ++++++++-------------- tooling/nargo_cli/src/cli/test_cmd.rs | 17 ++++-- tooling/nargo_cli/tests/stdlib-tests.rs | 13 +++- 10 files changed, 136 insertions(+), 130 deletions(-) create mode 100644 tooling/nargo/src/foreign_calls/default.rs diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 72ae6695b82..56fcb0325c3 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -3,6 +3,7 @@ use std::future::{self, Future}; use crate::insert_all_files_for_workspace_into_file_manager; use async_lsp::{ErrorCode, ResponseError}; use nargo::{ + foreign_calls::DefaultForeignCallExecutor, ops::{run_test, TestStatus}, PrintOutput, }; @@ -88,10 +89,16 @@ fn on_test_run_request_inner( &mut context, &test_function, PrintOutput::Stdout, - None, - Some(workspace.root_dir.clone()), - Some(package.name.to_string()), &CompileOptions::default(), + |output, base| { + DefaultForeignCallExecutor::with_base( + base, + output, + None, + Some(workspace.root_dir.clone()), + Some(package.name.to_string()), + ) + }, ); let result = match test_result { TestStatus::Pass => NargoTestRunResult { diff --git a/tooling/nargo/Cargo.toml b/tooling/nargo/Cargo.toml index a964ac5c071..9aa5f3ba530 100644 --- a/tooling/nargo/Cargo.toml +++ b/tooling/nargo/Cargo.toml @@ -27,21 +27,20 @@ tracing.workspace = true walkdir = "2.5.0" # Some dependencies are optional so we can compile to Wasm. -rand = { workspace = true, optional = true } serde = { workspace = true, optional = true } jsonrpsee = { workspace = true, optional = true } tokio = { workspace = true, optional = true } +rand = { workspace = true, optional = true } [target.'cfg(not(target_arch = "wasm32"))'.dependencies] -noir_fuzzer = { workspace = true, optional = true } -proptest = { workspace = true, optional = true } +noir_fuzzer = { workspace = true } +proptest = { workspace = true } [dev-dependencies] jsonrpsee = { workspace = true, features = ["server"] } [features] -default = ["execute", "test"] +default = ["rpc"] # Execution currently uses HTTP based Oracle resolvers; does not compile to Wasm. -execute = ["jsonrpsee/http-client", "jsonrpsee/macros", "tokio/rt", "serde", "rand"] -test = ["execute", "noir_fuzzer", "proptest"] +rpc = ["jsonrpsee/http-client", "jsonrpsee/macros", "tokio/rt", "serde", "rand"] diff --git a/tooling/nargo/src/foreign_calls/default.rs b/tooling/nargo/src/foreign_calls/default.rs new file mode 100644 index 00000000000..2cce946e3c2 --- /dev/null +++ b/tooling/nargo/src/foreign_calls/default.rs @@ -0,0 +1,59 @@ +use std::path::PathBuf; + +use acvm::AcirField; +use rand::Rng; +use serde::{Deserialize, Serialize}; + +use crate::PrintOutput; + +use super::{ + layers::{self, Layer, Layering}, + mocker::MockForeignCallExecutor, + print::PrintForeignCallExecutor, + rpc::RPCForeignCallExecutor, + ForeignCallExecutor, +}; + +pub struct DefaultForeignCallExecutor; + +impl DefaultForeignCallExecutor { + #[allow(clippy::new_ret_no_self)] + pub fn new<'a, F>( + output: PrintOutput<'a>, + resolver_url: Option<&str>, + root_path: Option, + package_name: Option, + ) -> impl ForeignCallExecutor + 'a + where + F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a, + { + Self::with_base(layers::Empty, output, resolver_url, root_path, package_name) + } + + pub fn with_base<'a, F, B>( + base: B, + output: PrintOutput<'a>, + resolver_url: Option<&str>, + root_path: Option, + package_name: Option, + ) -> DefaultForeignCallLayers<'a, B, F> + where + F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a, + B: ForeignCallExecutor + 'a, + { + // Adding them in the opposite order, so print is the outermost layer. + base.add_layer(resolver_url.map(|resolver_url| { + let id = rand::thread_rng().gen(); + RPCForeignCallExecutor::new(resolver_url, id, root_path, package_name) + })) + .add_layer(MockForeignCallExecutor::default()) + .add_layer(PrintForeignCallExecutor::new(output)) + } +} + +/// Facilitate static typing of layers on a base layer, so inner layers can be accessed. +pub type DefaultForeignCallLayers<'a, B, F> = Layer< + PrintForeignCallExecutor<'a>, + Layer, Layer, B, F>, F>, + F, +>; diff --git a/tooling/nargo/src/foreign_calls/mocker.rs b/tooling/nargo/src/foreign_calls/mocker.rs index 59ce7479cd4..42d788fcfc6 100644 --- a/tooling/nargo/src/foreign_calls/mocker.rs +++ b/tooling/nargo/src/foreign_calls/mocker.rs @@ -6,7 +6,6 @@ use acvm::{ AcirField, }; use noirc_printable_type::{decode_string_value, ForeignCallError}; -use serde::{Deserialize, Serialize}; use super::{ForeignCall, ForeignCallExecutor}; @@ -82,7 +81,7 @@ impl MockForeignCallExecutor { impl ForeignCallExecutor for MockForeignCallExecutor where - F: AcirField + Serialize + for<'a> Deserialize<'a>, + F: AcirField, { fn execute( &mut self, diff --git a/tooling/nargo/src/foreign_calls/mod.rs b/tooling/nargo/src/foreign_calls/mod.rs index 72311558250..a47a2ba8db8 100644 --- a/tooling/nargo/src/foreign_calls/mod.rs +++ b/tooling/nargo/src/foreign_calls/mod.rs @@ -1,18 +1,16 @@ -use std::path::PathBuf; - -use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField}; -use layers::{Layer, Layering}; -use mocker::MockForeignCallExecutor; +use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo}; use noirc_printable_type::ForeignCallError; -use print::{PrintForeignCallExecutor, PrintOutput}; -use rand::Rng; -use rpc::RPCForeignCallExecutor; -use serde::{Deserialize, Serialize}; pub mod layers; -pub(crate) mod mocker; -pub(crate) mod print; -pub(crate) mod rpc; +pub mod mocker; +pub mod print; + +#[cfg(feature = "rpc")] +pub mod default; +#[cfg(feature = "rpc")] +pub mod rpc; +#[cfg(feature = "rpc")] +pub use default::DefaultForeignCallExecutor; pub trait ForeignCallExecutor { fn execute( @@ -65,47 +63,3 @@ impl ForeignCall { } } } - -pub struct DefaultForeignCallExecutor; - -impl DefaultForeignCallExecutor { - #[allow(clippy::new_ret_no_self)] - pub fn new<'a, F>( - output: PrintOutput<'a>, - resolver_url: Option<&str>, - root_path: Option, - package_name: Option, - ) -> impl ForeignCallExecutor + 'a - where - F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a, - { - Self::with_base(layers::Empty, output, resolver_url, root_path, package_name) - } - - pub fn with_base<'a, F, B>( - base: B, - output: PrintOutput<'a>, - resolver_url: Option<&str>, - root_path: Option, - package_name: Option, - ) -> DefaultForeignCallLayers<'a, B, F> - where - F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a, - B: ForeignCallExecutor + 'a, - { - // Adding them in the opposite order, so print is the outermost layer. - base.add_layer(resolver_url.map(|resolver_url| { - let id = rand::thread_rng().gen(); - RPCForeignCallExecutor::new(resolver_url, id, root_path, package_name) - })) - .add_layer(MockForeignCallExecutor::default()) - .add_layer(PrintForeignCallExecutor::new(output)) - } -} - -/// Facilitate static typing of layers on a base layer, so inner layers can be accessed. -pub type DefaultForeignCallLayers<'a, B, F> = Layer< - PrintForeignCallExecutor<'a>, - Layer, Layer, B, F>, F>, - F, ->; diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index 694d4846536..5a226449458 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -9,15 +9,12 @@ pub mod constants; pub mod errors; +pub mod foreign_calls; pub mod ops; pub mod package; pub mod workspace; -#[cfg(feature = "execute")] -pub mod foreign_calls; - pub use self::errors::NargoError; -#[cfg(feature = "execute")] pub use self::foreign_calls::print::PrintOutput; use std::{ diff --git a/tooling/nargo/src/ops/mod.rs b/tooling/nargo/src/ops/mod.rs index e6540eed834..7a52a829be3 100644 --- a/tooling/nargo/src/ops/mod.rs +++ b/tooling/nargo/src/ops/mod.rs @@ -6,17 +6,12 @@ pub use self::compile::{ pub use self::optimize::{optimize_contract, optimize_program}; pub use self::transform::{transform_contract, transform_program}; -#[cfg(feature = "execute")] pub use self::execute::{execute_program, execute_program_with_profiling}; -#[cfg(feature = "test")] pub use self::test::{run_test, TestStatus}; mod check; mod compile; -mod optimize; -mod transform; - -#[cfg(feature = "execute")] mod execute; -#[cfg(feature = "test")] +mod optimize; mod test; +mod transform; diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 56afbbf29ec..660770e6ae1 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -1,5 +1,3 @@ -use std::path::PathBuf; - use acvm::{ acir::{ brillig::ForeignCallResult, @@ -13,14 +11,10 @@ use noirc_driver::{compile_no_check, CompileError, CompileOptions, DEFAULT_EXPRE use noirc_errors::{debug_info::DebugInfo, FileDiagnostic}; use noirc_frontend::hir::{def_map::TestFunction, Context}; use noirc_printable_type::ForeignCallError; -use serde::{Deserialize, Serialize}; use crate::{ errors::try_to_diagnose_runtime_error, - foreign_calls::{ - layers, print::PrintOutput, DefaultForeignCallExecutor, DefaultForeignCallLayers, - ForeignCallExecutor, - }, + foreign_calls::{layers, print::PrintOutput, ForeignCallExecutor}, NargoError, }; @@ -41,16 +35,19 @@ impl TestStatus { } #[allow(clippy::too_many_arguments)] -pub fn run_test>( +pub fn run_test<'a, B, F, E>( blackbox_solver: &B, context: &mut Context, test_function: &TestFunction, - output: PrintOutput<'_>, - foreign_call_resolver_url: Option<&str>, - root_path: Option, - package_name: Option, + output: PrintOutput<'a>, config: &CompileOptions, -) -> TestStatus { + foreign_call_executor: F, +) -> TestStatus +where + B: BlackBoxFunctionSolver, + F: Fn(PrintOutput<'a>, layers::Unhandled) -> E + 'a, + E: ForeignCallExecutor, +{ let test_function_has_no_arguments = context .def_interner .function_meta(&test_function.get_id()) @@ -67,12 +64,8 @@ pub fn run_test>( if test_function_has_no_arguments { // Run the backend to ensure the PWG evaluates functions like std::hash::pedersen, // otherwise constraints involving these expressions will not error. - let mut foreign_call_executor = TestForeignCallExecutor::new( - output, - foreign_call_resolver_url, - root_path, - package_name, - ); + let mut foreign_call_executor = + TestForeignCallExecutor::new(output, &foreign_call_executor); let circuit_execution = execute_program( &compiled_program.program, @@ -134,11 +127,9 @@ pub fn run_test>( program, initial_witness, blackbox_solver, - &mut TestForeignCallExecutor::::new( + &mut TestForeignCallExecutor::new( PrintOutput::None, - foreign_call_resolver_url, - root_path.clone(), - package_name.clone(), + &foreign_call_executor, ), ); @@ -275,37 +266,28 @@ fn check_expected_failure_message( } /// A specialized foreign call executor which tracks whether it has encountered any unknown foreign calls -struct TestForeignCallExecutor<'a, F> { - executor: DefaultForeignCallLayers<'a, layers::Unhandled, F>, +struct TestForeignCallExecutor { + executor: E, encountered_unknown_foreign_call: bool, } -impl<'a, F> TestForeignCallExecutor<'a, F> -where - F: Default + AcirField + Serialize + for<'de> Deserialize<'de> + 'a, -{ +impl TestForeignCallExecutor { #[allow(clippy::new_ret_no_self)] - fn new( - output: PrintOutput<'a>, - resolver_url: Option<&str>, - root_path: Option, - package_name: Option, - ) -> Self { + fn new<'a, F>(output: PrintOutput<'a>, foreign_call_executor: &F) -> Self + where + F: Fn(PrintOutput<'a>, layers::Unhandled) -> E + 'a, + { // Use a base layer that doesn't handle anything, which we handle in the `execute` below. - let executor = DefaultForeignCallExecutor::with_base( - layers::Unhandled, - output, - resolver_url, - root_path, - package_name, - ); + let executor = foreign_call_executor(output, layers::Unhandled); Self { executor, encountered_unknown_foreign_call: false } } } -impl<'a, F: AcirField + Serialize + for<'b> Deserialize<'b>> ForeignCallExecutor - for TestForeignCallExecutor<'a, F> +impl ForeignCallExecutor for TestForeignCallExecutor +where + F: AcirField, + E: ForeignCallExecutor, { fn execute( &mut self, diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 1fd4ed2d873..f4f50acd37c 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -14,8 +14,9 @@ use clap::Args; use fm::FileManager; use formatters::{Formatter, JsonFormatter, PrettyFormatter, TerseFormatter}; use nargo::{ - insert_all_files_for_workspace_into_file_manager, ops::TestStatus, package::Package, parse_all, - prepare_package, workspace::Workspace, PrintOutput, + foreign_calls::DefaultForeignCallExecutor, insert_all_files_for_workspace_into_file_manager, + ops::TestStatus, package::Package, parse_all, prepare_package, workspace::Workspace, + PrintOutput, }; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml}; use noirc_driver::{check_crate, CompileOptions, NOIR_ARTIFACT_VERSION_STRING}; @@ -494,10 +495,16 @@ impl<'a> TestRunner<'a> { &mut context, test_function, PrintOutput::String(&mut output_string), - foreign_call_resolver_url, - root_path, - Some(package_name), &self.args.compile_options, + |output, base| { + DefaultForeignCallExecutor::with_base( + base, + output, + foreign_call_resolver_url, + root_path.clone(), + Some(package_name.clone()), + ) + }, ); (test_status, output_string) } diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index 29b871814b8..bd08b0fd156 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -2,6 +2,7 @@ #![allow(clippy::items_after_test_module)] use clap::Parser; use fm::FileManager; +use nargo::foreign_calls::DefaultForeignCallExecutor; use nargo::PrintOutput; use noirc_driver::{check_crate, file_manager_with_stdlib, CompileOptions}; use noirc_frontend::hir::FunctionNameMatch; @@ -88,10 +89,16 @@ fn run_stdlib_tests(force_brillig: bool, inliner_aggressiveness: i64) { &mut context, &test_function, PrintOutput::Stdout, - None, - Some(dummy_package.root_dir.clone()), - Some(dummy_package.name.to_string()), &CompileOptions { force_brillig, inliner_aggressiveness, ..Default::default() }, + |output, base| { + DefaultForeignCallExecutor::with_base( + base, + output, + None, + Some(dummy_package.root_dir.clone()), + Some(dummy_package.name.to_string()), + ) + }, ); (test_name, status) })