From e41bfaf855bd3a173604ed8721edb2980b9e34e3 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Thu, 19 Dec 2024 14:57:03 +0000 Subject: [PATCH] Disable mocks in execution --- tooling/acvm_cli/src/cli/execute_cmd.rs | 8 +++-- tooling/lsp/src/requests/test_run.rs | 1 + tooling/nargo/src/foreign_calls/default.rs | 42 ++++++++++++++++++---- tooling/nargo/src/foreign_calls/layers.rs | 22 ++++++++++++ tooling/nargo/src/foreign_calls/mocker.rs | 9 ++--- tooling/nargo_cli/src/cli/execute_cmd.rs | 12 ++++--- tooling/nargo_cli/src/cli/test_cmd.rs | 1 + tooling/nargo_cli/tests/stdlib-tests.rs | 2 +- 8 files changed, 75 insertions(+), 22 deletions(-) diff --git a/tooling/acvm_cli/src/cli/execute_cmd.rs b/tooling/acvm_cli/src/cli/execute_cmd.rs index c3c83a8f000..dfbbe0b3918 100644 --- a/tooling/acvm_cli/src/cli/execute_cmd.rs +++ b/tooling/acvm_cli/src/cli/execute_cmd.rs @@ -74,8 +74,12 @@ pub(crate) fn execute_program_from_witness( &program, inputs_map, &Bn254BlackBoxSolver, - &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..8fc25bb004e 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,9 +15,9 @@ 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")] @@ -26,6 +26,18 @@ pub struct DefaultForeignCallBuilder<'a> { pub package_name: Option, } +impl<'a> Default for DefaultForeignCallBuilder<'a> { + fn default() -> Self { + Self { + output: PrintOutput::default(), + enable_mocks: true, + resolver_url: None, + root_path: None, + package_name: None, + } + } +} + impl<'a> DefaultForeignCallBuilder<'a> { /// Override the output. pub fn with_output(mut self, output: PrintOutput<'a>) -> Self { @@ -33,6 +45,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 +87,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 +100,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 +133,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 25df4e904aa..bc0536ad4b5 100644 --- a/tooling/nargo/src/foreign_calls/layers.rs +++ b/tooling/nargo/src/foreign_calls/layers.rs @@ -122,6 +122,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 42d788fcfc6..ac821155eef 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, diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 709caf71185..52b8e0fd59e 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}; @@ -141,12 +141,14 @@ pub(crate) fn execute_program( &compiled_program.program, initial_witness, &Bn254BlackBoxSolver, - &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 6aae94f6645..6215aae90e4 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -91,7 +91,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)