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!: nargo::ops::test::run_test generic in ForeignCallExecutor #6858

Merged
merged 2 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 10 additions & 3 deletions tooling/lsp/src/requests/test_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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 {
Expand Down
11 changes: 5 additions & 6 deletions tooling/nargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
59 changes: 59 additions & 0 deletions tooling/nargo/src/foreign_calls/default.rs
Original file line number Diff line number Diff line change
@@ -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<PathBuf>,
package_name: Option<String>,
) -> impl ForeignCallExecutor<F> + '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<PathBuf>,
package_name: Option<String>,
) -> DefaultForeignCallLayers<'a, B, F>
where
F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a,
B: ForeignCallExecutor<F> + '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<MockForeignCallExecutor<F>, Layer<Option<RPCForeignCallExecutor>, B, F>, F>,
F,
>;
3 changes: 1 addition & 2 deletions tooling/nargo/src/foreign_calls/mocker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use acvm::{
AcirField,
};
use noirc_printable_type::{decode_string_value, ForeignCallError};
use serde::{Deserialize, Serialize};

use super::{ForeignCall, ForeignCallExecutor};

Expand Down Expand Up @@ -82,7 +81,7 @@ impl<F: AcirField> MockForeignCallExecutor<F> {

impl<F> ForeignCallExecutor<F> for MockForeignCallExecutor<F>
where
F: AcirField + Serialize + for<'a> Deserialize<'a>,
F: AcirField,
{
fn execute(
&mut self,
Expand Down
66 changes: 10 additions & 56 deletions tooling/nargo/src/foreign_calls/mod.rs
Original file line number Diff line number Diff line change
@@ -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<F> {
fn execute(
Expand Down Expand Up @@ -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<PathBuf>,
package_name: Option<String>,
) -> impl ForeignCallExecutor<F> + '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<PathBuf>,
package_name: Option<String>,
) -> DefaultForeignCallLayers<'a, B, F>
where
F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a,
B: ForeignCallExecutor<F> + '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<MockForeignCallExecutor<F>, Layer<Option<RPCForeignCallExecutor>, B, F>, F>,
F,
>;
5 changes: 1 addition & 4 deletions tooling/nargo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down
9 changes: 2 additions & 7 deletions tooling/nargo/src/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
70 changes: 26 additions & 44 deletions tooling/nargo/src/ops/test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::path::PathBuf;

use acvm::{
acir::{
brillig::ForeignCallResult,
Expand All @@ -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,
};

Expand All @@ -41,16 +35,19 @@ impl TestStatus {
}

#[allow(clippy::too_many_arguments)]
pub fn run_test<B: BlackBoxFunctionSolver<FieldElement>>(
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<PathBuf>,
package_name: Option<String>,
output: PrintOutput<'a>,
config: &CompileOptions,
) -> TestStatus {
foreign_call_executor: F,
) -> TestStatus
where
B: BlackBoxFunctionSolver<FieldElement>,
F: Fn(PrintOutput<'a>, layers::Unhandled) -> E + 'a,
E: ForeignCallExecutor<FieldElement>,
{
let test_function_has_no_arguments = context
.def_interner
.function_meta(&test_function.get_id())
Expand All @@ -67,12 +64,8 @@ pub fn run_test<B: BlackBoxFunctionSolver<FieldElement>>(
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,
Expand Down Expand Up @@ -134,11 +127,9 @@ pub fn run_test<B: BlackBoxFunctionSolver<FieldElement>>(
program,
initial_witness,
blackbox_solver,
&mut TestForeignCallExecutor::<FieldElement>::new(
&mut TestForeignCallExecutor::new(
PrintOutput::None,
foreign_call_resolver_url,
root_path.clone(),
package_name.clone(),
&foreign_call_executor,
),
);

Expand Down Expand Up @@ -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<E> {
executor: E,
encountered_unknown_foreign_call: bool,
}

impl<'a, F> TestForeignCallExecutor<'a, F>
where
F: Default + AcirField + Serialize + for<'de> Deserialize<'de> + 'a,
{
impl<E> TestForeignCallExecutor<E> {
#[allow(clippy::new_ret_no_self)]
fn new(
output: PrintOutput<'a>,
resolver_url: Option<&str>,
root_path: Option<PathBuf>,
package_name: Option<String>,
) -> 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<F>
for TestForeignCallExecutor<'a, F>
impl<E, F> ForeignCallExecutor<F> for TestForeignCallExecutor<E>
where
F: AcirField,
E: ForeignCallExecutor<F>,
{
fn execute(
&mut self,
Expand Down
Loading
Loading