diff --git a/test_programs/execution_failure/mocks_in_execution/Nargo.toml b/test_programs/execution_failure/mocks_in_execution/Nargo.toml new file mode 100644 index 00000000000..c83da9a3d0c --- /dev/null +++ b/test_programs/execution_failure/mocks_in_execution/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "mocks_in_execution" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] diff --git a/test_programs/execution_failure/mocks_in_execution/Prover.toml b/test_programs/execution_failure/mocks_in_execution/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test_programs/execution_failure/mocks_in_execution/src/main.nr b/test_programs/execution_failure/mocks_in_execution/src/main.nr new file mode 100644 index 00000000000..b43068ab251 --- /dev/null +++ b/test_programs/execution_failure/mocks_in_execution/src/main.nr @@ -0,0 +1,5 @@ +fn main() { + // Trying to use a mock in `nargo execute` should fail. + let mock = unsafe { std::test::OracleMock::mock("foo") }; + assert_eq(mock.id, 0); +} diff --git a/test_programs/execution_success/brillig_oracle/Prover.toml b/test_programs/execution_success/brillig_oracle/Prover.toml deleted file mode 100644 index 161f4fb62c0..00000000000 --- a/test_programs/execution_success/brillig_oracle/Prover.toml +++ /dev/null @@ -1,2 +0,0 @@ -_x = "10" - diff --git a/test_programs/execution_success/brillig_oracle/Nargo.toml b/test_programs/noir_test_success/brillig_oracle/Nargo.toml similarity index 100% rename from test_programs/execution_success/brillig_oracle/Nargo.toml rename to test_programs/noir_test_success/brillig_oracle/Nargo.toml diff --git a/test_programs/execution_success/brillig_oracle/src/main.nr b/test_programs/noir_test_success/brillig_oracle/src/main.nr similarity index 97% rename from test_programs/execution_success/brillig_oracle/src/main.nr rename to test_programs/noir_test_success/brillig_oracle/src/main.nr index cae174edb99..77dbeef9aa1 100644 --- a/test_programs/execution_success/brillig_oracle/src/main.nr +++ b/test_programs/noir_test_success/brillig_oracle/src/main.nr @@ -1,6 +1,11 @@ use std::slice; use std::test::OracleMock; +#[test] +fn test_main() { + main(10); +} + // Tests oracle usage in brillig/unconstrained functions fn main(_x: Field) { /// Safety: testing context @@ -46,4 +51,3 @@ unconstrained fn get_number_sequence_wrapper(size: Field) { assert(slice[i] == reversed_slice[19 - i]); } } - diff --git a/tooling/acvm_cli/src/cli/execute_cmd.rs b/tooling/acvm_cli/src/cli/execute_cmd.rs index 9fed8cbd497..e5d48073ca8 100644 --- a/tooling/acvm_cli/src/cli/execute_cmd.rs +++ b/tooling/acvm_cli/src/cli/execute_cmd.rs @@ -82,8 +82,12 @@ pub(crate) fn execute_program_from_witness( &program, inputs_map, &Bn254BlackBoxSolver(pedantic_solving), - &mut DefaultForeignCallBuilder { output: PrintOutput::Stdout, ..Default::default() } - .build(), + &mut DefaultForeignCallBuilder { + output: PrintOutput::Stdout, + enable_mocks: false, + ..Default::default() + } + .build(), ) .map_err(CliError::CircuitExecutionError) } diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 1071866dfad..bd53526298e 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -93,6 +93,7 @@ fn on_test_run_request_inner( |output, base| { DefaultForeignCallBuilder { output, + enable_mocks: true, resolver_url: None, // NB without this the root and package don't do anything. root_path: Some(workspace.root_dir.clone()), package_name: Some(package.name.to_string()), diff --git a/tooling/nargo/src/foreign_calls/default.rs b/tooling/nargo/src/foreign_calls/default.rs index ce4af3aa744..19928e89563 100644 --- a/tooling/nargo/src/foreign_calls/default.rs +++ b/tooling/nargo/src/foreign_calls/default.rs @@ -4,8 +4,8 @@ use serde::{Deserialize, Serialize}; use crate::PrintOutput; use super::{ - layers::{self, Layer, Layering}, - mocker::MockForeignCallExecutor, + layers::{self, Either, Layer, Layering}, + mocker::{DisabledMockForeignCallExecutor, MockForeignCallExecutor}, print::PrintForeignCallExecutor, ForeignCallExecutor, }; @@ -15,17 +15,38 @@ use super::rpc::RPCForeignCallExecutor; /// A builder for [DefaultForeignCallLayers] where we can enable fields based on feature flags, /// which is easier than providing different overrides for a `new` method. -#[derive(Default)] pub struct DefaultForeignCallBuilder<'a> { pub output: PrintOutput<'a>, + pub enable_mocks: bool, + #[cfg(feature = "rpc")] pub resolver_url: Option, + #[cfg(feature = "rpc")] pub root_path: Option, + #[cfg(feature = "rpc")] pub package_name: Option, } +impl<'a> Default for DefaultForeignCallBuilder<'a> { + fn default() -> Self { + Self { + output: PrintOutput::default(), + enable_mocks: true, + + #[cfg(feature = "rpc")] + resolver_url: None, + + #[cfg(feature = "rpc")] + root_path: None, + + #[cfg(feature = "rpc")] + package_name: None, + } + } +} + impl<'a> DefaultForeignCallBuilder<'a> { /// Override the output. pub fn with_output(mut self, output: PrintOutput<'a>) -> Self { @@ -33,6 +54,12 @@ impl<'a> DefaultForeignCallBuilder<'a> { self } + /// Enable or disable mocks. + pub fn with_mocks(mut self, enabled: bool) -> Self { + self.enable_mocks = enabled; + self + } + /// Compose the executor layers with [layers::Empty] as the default handler. pub fn build(self) -> DefaultForeignCallLayers<'a, layers::Empty, F> where @@ -69,7 +96,11 @@ impl<'a> DefaultForeignCallBuilder<'a> { }; executor - .add_layer(MockForeignCallExecutor::default()) + .add_layer(if self.enable_mocks { + Either::Left(MockForeignCallExecutor::default()) + } else { + Either::Right(DisabledMockForeignCallExecutor) + }) .add_layer(PrintForeignCallExecutor::new(self.output)) } } @@ -78,11 +109,16 @@ impl<'a> DefaultForeignCallBuilder<'a> { #[cfg(feature = "rpc")] pub type DefaultForeignCallLayers<'a, B, F> = Layer< PrintForeignCallExecutor<'a>, - Layer, Layer, B>>, + Layer< + Either, DisabledMockForeignCallExecutor>, + Layer, B>, + >, >; #[cfg(not(feature = "rpc"))] -pub type DefaultForeignCallLayers<'a, B, F> = - Layer, Layer, B>>; +pub type DefaultForeignCallLayers<'a, B, F> = Layer< + PrintForeignCallExecutor<'a>, + Layer, DisabledMockForeignCallExecutor>, B>, +>; /// Convenience constructor for code that used to create the executor this way. #[cfg(feature = "rpc")] @@ -106,6 +142,7 @@ impl DefaultForeignCallExecutor { { DefaultForeignCallBuilder { output, + enable_mocks: true, resolver_url: resolver_url.map(|s| s.to_string()), root_path, package_name, diff --git a/tooling/nargo/src/foreign_calls/layers.rs b/tooling/nargo/src/foreign_calls/layers.rs index 19f14c6f4a8..83145cacb44 100644 --- a/tooling/nargo/src/foreign_calls/layers.rs +++ b/tooling/nargo/src/foreign_calls/layers.rs @@ -121,6 +121,28 @@ impl Layering for T { } } +/// A case where we can have either this or that type of handler. +pub enum Either { + Left(L), + Right(R), +} + +impl ForeignCallExecutor for Either +where + L: ForeignCallExecutor, + R: ForeignCallExecutor, +{ + fn execute( + &mut self, + foreign_call: &ForeignCallWaitInfo, + ) -> Result, ForeignCallError> { + match self { + Either::Left(left) => left.execute(foreign_call), + Either::Right(right) => right.execute(foreign_call), + } + } +} + /// Support disabling a layer by making it optional. /// This way we can still have a known static type for a composition, /// because layers are always added, potentially wrapped in an `Option`. diff --git a/tooling/nargo/src/foreign_calls/mocker.rs b/tooling/nargo/src/foreign_calls/mocker.rs index b289e907cd7..41fac610052 100644 --- a/tooling/nargo/src/foreign_calls/mocker.rs +++ b/tooling/nargo/src/foreign_calls/mocker.rs @@ -1,5 +1,3 @@ -use std::marker::PhantomData; - use acvm::{ acir::brillig::{ForeignCallParam, ForeignCallResult}, pwg::ForeignCallWaitInfo, @@ -178,12 +176,9 @@ where } /// Handler that panics if any of the mock functions are called. -#[allow(dead_code)] // TODO: Make the mocker optional -pub(crate) struct DisabledMockForeignCallExecutor { - _field: PhantomData, -} +pub struct DisabledMockForeignCallExecutor; -impl ForeignCallExecutor for DisabledMockForeignCallExecutor { +impl ForeignCallExecutor for DisabledMockForeignCallExecutor { fn execute( &mut self, foreign_call: &ForeignCallWaitInfo, @@ -198,7 +193,8 @@ impl ForeignCallExecutor for DisabledMockForeignCallExecutor { | ForeignCall::ClearMock, ) = ForeignCall::lookup(foreign_call_name) { - panic!("unexpected mock call: {}", foreign_call.function) + // Returning an error instead of panicking so this can be tested. + return Err(ForeignCallError::Disabled(foreign_call.function.to_string())); } Err(ForeignCallError::NoHandler(foreign_call.function.clone())) } diff --git a/tooling/nargo/src/foreign_calls/mod.rs b/tooling/nargo/src/foreign_calls/mod.rs index 06fe42bfec9..f17a97cecd4 100644 --- a/tooling/nargo/src/foreign_calls/mod.rs +++ b/tooling/nargo/src/foreign_calls/mod.rs @@ -66,6 +66,9 @@ impl ForeignCall { #[derive(Debug, Error)] pub enum ForeignCallError { + #[error("Attempted to call disabled foreign call `{0}`")] + Disabled(String), + #[error("No handler could be found for foreign call `{0}`")] NoHandler(String), diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 88e74340c71..cb471995be5 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -7,7 +7,7 @@ use clap::Args; use nargo::constants::PROVER_INPUT_FILE; use nargo::errors::try_to_diagnose_runtime_error; -use nargo::foreign_calls::DefaultForeignCallExecutor; +use nargo::foreign_calls::DefaultForeignCallBuilder; use nargo::package::Package; use nargo::PrintOutput; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml}; @@ -150,12 +150,14 @@ pub(crate) fn execute_program( &compiled_program.program, initial_witness, &Bn254BlackBoxSolver(pedantic_solving), - &mut DefaultForeignCallExecutor::new( - PrintOutput::Stdout, - foreign_call_resolver_url, + &mut DefaultForeignCallBuilder { + output: PrintOutput::Stdout, + enable_mocks: false, + resolver_url: foreign_call_resolver_url.map(|s| s.to_string()), root_path, package_name, - ), + } + .build(), ); match solved_witness_stack_err { Ok(solved_witness_stack) => Ok(solved_witness_stack), diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 43f1c306d88..9bf3ae9fedf 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -499,6 +499,7 @@ impl<'a> TestRunner<'a> { |output, base| { DefaultForeignCallBuilder { output, + enable_mocks: true, resolver_url: foreign_call_resolver_url.map(|s| s.to_string()), root_path: root_path.clone(), package_name: Some(package_name.clone()), diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index b162b282781..048de33f24c 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -92,7 +92,7 @@ fn run_stdlib_tests(force_brillig: bool, inliner_aggressiveness: i64) { PrintOutput::Stdout, &CompileOptions { force_brillig, inliner_aggressiveness, ..Default::default() }, |output, base| { - DefaultForeignCallBuilder { output, ..Default::default() }.build_with_base(base) + DefaultForeignCallBuilder::default().with_output(output).build_with_base(base) }, ); (test_name, status)