Skip to content

Commit

Permalink
Disable mocks in execution
Browse files Browse the repository at this point in the history
  • Loading branch information
aakoshh committed Dec 19, 2024
1 parent 005b30f commit e41bfaf
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 22 deletions.
8 changes: 6 additions & 2 deletions tooling/acvm_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
1 change: 1 addition & 0 deletions tooling/lsp/src/requests/test_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
42 changes: 35 additions & 7 deletions tooling/nargo/src/foreign_calls/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand All @@ -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<String>,
#[cfg(feature = "rpc")]
Expand All @@ -26,13 +26,31 @@ pub struct DefaultForeignCallBuilder<'a> {
pub package_name: Option<String>,
}

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 {
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
Expand Down Expand Up @@ -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))
}
}
Expand All @@ -78,11 +100,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")]
Expand All @@ -106,6 +133,7 @@ impl DefaultForeignCallExecutor {
{
DefaultForeignCallBuilder {
output,
enable_mocks: true,
resolver_url: resolver_url.map(|s| s.to_string()),
root_path,
package_name,
Expand Down
22 changes: 22 additions & 0 deletions tooling/nargo/src/foreign_calls/layers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,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`.
Expand Down
9 changes: 2 additions & 7 deletions tooling/nargo/src/foreign_calls/mocker.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::marker::PhantomData;

use acvm::{
acir::brillig::{ForeignCallParam, ForeignCallResult},
pwg::ForeignCallWaitInfo,
Expand Down Expand Up @@ -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>,
Expand Down
12 changes: 7 additions & 5 deletions tooling/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions tooling/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
2 changes: 1 addition & 1 deletion tooling/nargo_cli/tests/stdlib-tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit e41bfaf

Please sign in to comment.