feat!: Disable mocks in execute (#6869)

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]

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);
+}

test_programs/execution_success/brillig_oracle/src/main.nr → 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]);
     }
 }
-

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)
 }

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()),

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,24 +15,51 @@ 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<String>,
+
     #[cfg(feature = "rpc")]
     pub root_path: Option<std::path::PathBuf>,
+
     #[cfg(feature = "rpc")]
     pub package_name: Option<String>,
 }
 
+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 {
         self.output = output;
         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<F>(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<MockForeignCallExecutor<F>, Layer<Option<RPCForeignCallExecutor>, B>>,
+    Layer<
+        Either<MockForeignCallExecutor<F>, DisabledMockForeignCallExecutor>,
+        Layer<Option<RPCForeignCallExecutor>, B>,
+    >,
 >;
 #[cfg(not(feature = "rpc"))]
-pub type DefaultForeignCallLayers<'a, B, F> =
-    Layer<PrintForeignCallExecutor<'a>, Layer<MockForeignCallExecutor<F>, B>>;
+pub type DefaultForeignCallLayers<'a, B, F> = Layer<
+    PrintForeignCallExecutor<'a>,
+    Layer<Either<MockForeignCallExecutor<F>, 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,

tooling/nargo/src/foreign_calls/layers.rs

@@ -121,6 +121,28 @@ impl<T> Layering for T {
     }
 }
 
+/// A case where we can have either this or that type of handler.
+pub enum Either<L, R> {
+    Left(L),
+    Right(R),
+}
+
+impl<L, R, F> ForeignCallExecutor<F> for Either<L, R>
+where
+    L: ForeignCallExecutor<F>,
+    R: ForeignCallExecutor<F>,
+{
+    fn execute(
+        &mut self,
+        foreign_call: &ForeignCallWaitInfo<F>,
+    ) -> Result<ForeignCallResult<F>, 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`.

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<F> {
-    _field: PhantomData<F>,
-}
+pub struct DisabledMockForeignCallExecutor;
 
-impl<F> ForeignCallExecutor<F> for DisabledMockForeignCallExecutor<F> {
+impl<F> ForeignCallExecutor<F> for DisabledMockForeignCallExecutor {
     fn execute(
         &mut self,
         foreign_call: &ForeignCallWaitInfo<F>,
@@ -198,7 +193,8 @@ impl<F> ForeignCallExecutor<F> for DisabledMockForeignCallExecutor<F> {
             | 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()))
     }

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),
 

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),

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()),

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)