From 7e33e2afcff7b4d169fb513ed5d16efe3b66cddc Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 29 Nov 2024 19:18:11 +0000 Subject: [PATCH 1/5] chore: refactor foreign call executors --- compiler/noirc_printable_type/src/lib.rs | 3 + tooling/acvm_cli/src/cli/execute_cmd.rs | 2 +- tooling/debugger/src/foreign_calls.rs | 2 +- tooling/nargo/src/foreign_calls/mocker.rs | 176 +++++++ tooling/nargo/src/foreign_calls/mod.rs | 148 ++++++ tooling/nargo/src/foreign_calls/print.rs | 36 ++ tooling/nargo/src/foreign_calls/rpc.rs | 225 ++++++++ tooling/nargo/src/lib.rs | 1 + tooling/nargo/src/ops/execute.rs | 3 +- tooling/nargo/src/ops/foreign_calls.rs | 494 ------------------ tooling/nargo/src/ops/mod.rs | 2 - tooling/nargo/src/ops/test.rs | 6 +- tooling/nargo_cli/benches/criterion.rs | 2 +- tooling/nargo_cli/src/cli/execute_cmd.rs | 2 +- tooling/nargo_cli/src/cli/info_cmd.rs | 2 +- tooling/nargo_cli/tests/stdlib-props.rs | 3 +- .../src/cli/execution_flamegraph_cmd.rs | 2 +- 17 files changed, 602 insertions(+), 507 deletions(-) create mode 100644 tooling/nargo/src/foreign_calls/mocker.rs create mode 100644 tooling/nargo/src/foreign_calls/mod.rs create mode 100644 tooling/nargo/src/foreign_calls/print.rs create mode 100644 tooling/nargo/src/foreign_calls/rpc.rs delete mode 100644 tooling/nargo/src/ops/foreign_calls.rs diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 5ab04c6f576..838a2472125 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -69,6 +69,9 @@ pub enum PrintableValueDisplay { #[derive(Debug, Error)] pub enum ForeignCallError { + #[error("No handler could be found for foreign call `{0}`")] + NoHandler(String), + #[error("Foreign call inputs needed for execution are missing")] MissingForeignCallInputs, diff --git a/tooling/acvm_cli/src/cli/execute_cmd.rs b/tooling/acvm_cli/src/cli/execute_cmd.rs index c453936568c..bf5969718e5 100644 --- a/tooling/acvm_cli/src/cli/execute_cmd.rs +++ b/tooling/acvm_cli/src/cli/execute_cmd.rs @@ -8,7 +8,7 @@ use clap::Args; use crate::cli::fs::inputs::{read_bytecode_from_file, read_inputs_from_file}; use crate::errors::CliError; -use nargo::ops::{execute_program, DefaultForeignCallExecutor}; +use nargo::{foreign_calls::DefaultForeignCallExecutor, ops::execute_program}; use super::fs::witness::{create_output_witness_string, save_witness_to_dir}; diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs index 6a773a4b348..ecf27a22f29 100644 --- a/tooling/debugger/src/foreign_calls.rs +++ b/tooling/debugger/src/foreign_calls.rs @@ -3,7 +3,7 @@ use acvm::{ pwg::ForeignCallWaitInfo, AcirField, FieldElement, }; -use nargo::ops::{DefaultForeignCallExecutor, ForeignCallExecutor}; +use nargo::foreign_calls::{DefaultForeignCallExecutor, ForeignCallExecutor}; use noirc_artifacts::debug::{DebugArtifact, DebugVars, StackFrame}; use noirc_errors::debug_info::{DebugFnId, DebugVarId}; use noirc_printable_type::ForeignCallError; diff --git a/tooling/nargo/src/foreign_calls/mocker.rs b/tooling/nargo/src/foreign_calls/mocker.rs new file mode 100644 index 00000000000..c93d16bbaf6 --- /dev/null +++ b/tooling/nargo/src/foreign_calls/mocker.rs @@ -0,0 +1,176 @@ +use acvm::{ + acir::brillig::{ForeignCallParam, ForeignCallResult}, + pwg::ForeignCallWaitInfo, + AcirField, +}; +use noirc_printable_type::{decode_string_value, ForeignCallError}; +use serde::{Deserialize, Serialize}; + +use super::{ForeignCall, ForeignCallExecutor}; + +/// This struct represents an oracle mock. It can be used for testing programs that use oracles. +#[derive(Debug, PartialEq, Eq, Clone)] +struct MockedCall { + /// The id of the mock, used to update or remove it + id: usize, + /// The oracle it's mocking + name: String, + /// Optionally match the parameters + params: Option>>, + /// The parameters with which the mock was last called + last_called_params: Option>>, + /// The result to return when this mock is called + result: ForeignCallResult, + /// How many times should this mock be called before it is removed + times_left: Option, +} + +impl MockedCall { + fn new(id: usize, name: String) -> Self { + Self { + id, + name, + params: None, + last_called_params: None, + result: ForeignCallResult { values: vec![] }, + times_left: None, + } + } +} + +impl MockedCall { + fn matches(&self, name: &str, params: &[ForeignCallParam]) -> bool { + self.name == name && (self.params.is_none() || self.params.as_deref() == Some(params)) + } +} + +#[derive(Debug, Default)] +pub(crate) struct MockForeignCallExecutor { + /// Mocks have unique ids used to identify them in Noir, allowing to update or remove them. + last_mock_id: usize, + /// The registered mocks + mocked_responses: Vec>, +} + +impl MockForeignCallExecutor { + fn extract_mock_id( + foreign_call_inputs: &[ForeignCallParam], + ) -> Result<(usize, &[ForeignCallParam]), ForeignCallError> { + let (id, params) = + foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; + let id = + usize::try_from(id.unwrap_field().try_to_u64().expect("value does not fit into u64")) + .expect("value does not fit into usize"); + Ok((id, params)) + } + + fn find_mock_by_id(&self, id: usize) -> Option<&MockedCall> { + self.mocked_responses.iter().find(|response| response.id == id) + } + + fn find_mock_by_id_mut(&mut self, id: usize) -> Option<&mut MockedCall> { + self.mocked_responses.iter_mut().find(|response| response.id == id) + } + + fn parse_string(param: &ForeignCallParam) -> String { + let fields: Vec<_> = param.fields().to_vec(); + decode_string_value(&fields) + } +} + +impl Deserialize<'a>> ForeignCallExecutor + for MockForeignCallExecutor +{ + fn execute( + &mut self, + foreign_call: &ForeignCallWaitInfo, + ) -> Result, ForeignCallError> { + let foreign_call_name = foreign_call.function.as_str(); + match ForeignCall::lookup(foreign_call_name) { + Some(ForeignCall::CreateMock) => { + let mock_oracle_name = Self::parse_string(&foreign_call.inputs[0]); + assert!(ForeignCall::lookup(&mock_oracle_name).is_none()); + let id = self.last_mock_id; + self.mocked_responses.push(MockedCall::new(id, mock_oracle_name)); + self.last_mock_id += 1; + + Ok(F::from(id).into()) + } + Some(ForeignCall::SetMockParams) => { + let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; + self.find_mock_by_id_mut(id) + .unwrap_or_else(|| panic!("Unknown mock id {}", id)) + .params = Some(params.to_vec()); + + Ok(ForeignCallResult::default()) + } + Some(ForeignCall::GetMockLastParams) => { + let (id, _) = Self::extract_mock_id(&foreign_call.inputs)?; + let mock = + self.find_mock_by_id(id).unwrap_or_else(|| panic!("Unknown mock id {}", id)); + + let last_called_params = mock + .last_called_params + .clone() + .unwrap_or_else(|| panic!("Mock {} was never called", mock.name)); + + Ok(last_called_params.into()) + } + Some(ForeignCall::SetMockReturns) => { + let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; + self.find_mock_by_id_mut(id) + .unwrap_or_else(|| panic!("Unknown mock id {}", id)) + .result = ForeignCallResult { values: params.to_vec() }; + + Ok(ForeignCallResult::default()) + } + Some(ForeignCall::SetMockTimes) => { + let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; + let times = + params[0].unwrap_field().try_to_u64().expect("Invalid bit size of times"); + + self.find_mock_by_id_mut(id) + .unwrap_or_else(|| panic!("Unknown mock id {}", id)) + .times_left = Some(times); + + Ok(ForeignCallResult::default()) + } + Some(ForeignCall::ClearMock) => { + let (id, _) = Self::extract_mock_id(&foreign_call.inputs)?; + self.mocked_responses.retain(|response| response.id != id); + Ok(ForeignCallResult::default()) + } + _ => { + let mock_response_position = self + .mocked_responses + .iter() + .position(|response| response.matches(foreign_call_name, &foreign_call.inputs)); + + if let Some(response_position) = mock_response_position { + // If the program has registered a mocked response to this oracle call then we prefer responding + // with that. + + let mock = self + .mocked_responses + .get_mut(response_position) + .expect("Invalid position of mocked response"); + + mock.last_called_params = Some(foreign_call.inputs.clone()); + + let result = mock.result.values.clone(); + + if let Some(times_left) = &mut mock.times_left { + *times_left -= 1; + if *times_left == 0 { + self.mocked_responses.remove(response_position); + } + } + + Ok(result.into()) + } else { + Err(ForeignCallError::NoHandler(foreign_call_name.to_string())) + } + } + } + } +} diff --git a/tooling/nargo/src/foreign_calls/mod.rs b/tooling/nargo/src/foreign_calls/mod.rs new file mode 100644 index 00000000000..5e14757e24e --- /dev/null +++ b/tooling/nargo/src/foreign_calls/mod.rs @@ -0,0 +1,148 @@ +use std::path::PathBuf; + +use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField}; +use mocker::MockForeignCallExecutor; +use noirc_printable_type::ForeignCallError; +use print::PrintForeignCallExecutor; +use rand::Rng; +use rpc::RPCForeignCallExecutor; +use serde::{Deserialize, Serialize}; + +mod mocker; +mod print; +mod rpc; + +pub trait ForeignCallExecutor { + fn execute( + &mut self, + foreign_call: &ForeignCallWaitInfo, + ) -> Result, ForeignCallError>; +} + +/// This enumeration represents the Brillig foreign calls that are natively supported by nargo. +/// After resolution of a foreign call, nargo will restart execution of the ACVM +pub enum ForeignCall { + Print, + CreateMock, + SetMockParams, + GetMockLastParams, + SetMockReturns, + SetMockTimes, + ClearMock, +} + +impl std::fmt::Display for ForeignCall { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.name()) + } +} + +impl ForeignCall { + pub(crate) fn name(&self) -> &'static str { + match self { + ForeignCall::Print => "print", + ForeignCall::CreateMock => "create_mock", + ForeignCall::SetMockParams => "set_mock_params", + ForeignCall::GetMockLastParams => "get_mock_last_params", + ForeignCall::SetMockReturns => "set_mock_returns", + ForeignCall::SetMockTimes => "set_mock_times", + ForeignCall::ClearMock => "clear_mock", + } + } + + pub(crate) fn lookup(op_name: &str) -> Option { + match op_name { + "print" => Some(ForeignCall::Print), + "create_mock" => Some(ForeignCall::CreateMock), + "set_mock_params" => Some(ForeignCall::SetMockParams), + "get_mock_last_params" => Some(ForeignCall::GetMockLastParams), + "set_mock_returns" => Some(ForeignCall::SetMockReturns), + "set_mock_times" => Some(ForeignCall::SetMockTimes), + "clear_mock" => Some(ForeignCall::ClearMock), + _ => None, + } + } +} + +#[derive(Debug, Default)] +pub struct DefaultForeignCallExecutor { + /// The executor for any [`ForeignCall::Print`] calls. + printer: Option, + mocker: MockForeignCallExecutor, + external: Option, +} + +impl DefaultForeignCallExecutor { + pub fn new( + show_output: bool, + resolver_url: Option<&str>, + root_path: Option, + package_name: Option, + ) -> Self { + let id = rand::thread_rng().gen(); + let printer = if show_output { Some(PrintForeignCallExecutor) } else { None }; + let external_resolver = resolver_url.map(|resolver_url| { + RPCForeignCallExecutor::new(resolver_url, id, root_path, package_name) + }); + DefaultForeignCallExecutor { + + printer, + mocker: MockForeignCallExecutor::default(), + external: external_resolver, + } + } +} + +impl Deserialize<'a>> ForeignCallExecutor + for DefaultForeignCallExecutor +{ + fn execute( + &mut self, + foreign_call: &ForeignCallWaitInfo, + ) -> Result, ForeignCallError> { + let foreign_call_name = foreign_call.function.as_str(); + match ForeignCall::lookup(foreign_call_name) { + Some(ForeignCall::Print) => { + if let Some(printer) = &mut self.printer { + printer.execute(foreign_call) + } else { + Ok(ForeignCallResult::default()) + } + } + Some( + ForeignCall::CreateMock + | ForeignCall::SetMockParams + | ForeignCall::GetMockLastParams + | ForeignCall::SetMockReturns + | ForeignCall::SetMockTimes + | ForeignCall::ClearMock, + ) => self.mocker.execute(foreign_call), + + None => { + // First check if there's any defined mock responses for this foreign call. + match self.mocker.execute(foreign_call) { + Err(ForeignCallError::NoHandler(_)) => (), + response_or_error => return response_or_error + }; + + if let Some(external_resolver) = &mut self.external { + // If the user has registered an external resolver then we forward any remaining oracle calls there. + match external_resolver.execute(foreign_call) { + Err(ForeignCallError::NoHandler(_)) => (), + response_or_error => return response_or_error + }; + } + + // If all executors have no handler for the given foreign call then we cannot + // return a correct response to the ACVM. The best we can do is to return an empty response, + // this allows us to ignore any foreign calls which exist solely to pass information from inside + // the circuit to the environment (e.g. custom logging) as the execution will still be able to progress. + // + // We optimistically return an empty response for all oracle calls as the ACVM will error + // should a response have been required. + Ok(ForeignCallResult::default()) + + } + } + } +} diff --git a/tooling/nargo/src/foreign_calls/print.rs b/tooling/nargo/src/foreign_calls/print.rs new file mode 100644 index 00000000000..d0befa0bd0b --- /dev/null +++ b/tooling/nargo/src/foreign_calls/print.rs @@ -0,0 +1,36 @@ +use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField}; +use noirc_printable_type::{ForeignCallError, PrintableValueDisplay}; + +use super::{ForeignCall, ForeignCallExecutor}; + +#[derive(Debug, Default)] +pub(super) struct PrintForeignCallExecutor; + +impl ForeignCallExecutor for PrintForeignCallExecutor { + fn execute( + &mut self, + foreign_call: &ForeignCallWaitInfo, + ) -> Result, ForeignCallError> { + let foreign_call_name = foreign_call.function.as_str(); + match ForeignCall::lookup(foreign_call_name) { + Some(ForeignCall::Print) => { + let skip_newline = foreign_call.inputs[0].unwrap_field().is_zero(); + + let foreign_call_inputs = foreign_call + .inputs + .split_first() + .ok_or(ForeignCallError::MissingForeignCallInputs)? + .1; + + let display_values: PrintableValueDisplay = foreign_call_inputs.try_into()?; + let display_string = + format!("{display_values}{}", if skip_newline { "" } else { "\n" }); + + print!("{display_string}"); + + Ok(ForeignCallResult::default()) + } + _ => Err(ForeignCallError::NoHandler(foreign_call_name.to_string())), + } + } +} diff --git a/tooling/nargo/src/foreign_calls/rpc.rs b/tooling/nargo/src/foreign_calls/rpc.rs new file mode 100644 index 00000000000..b9ee7119d6b --- /dev/null +++ b/tooling/nargo/src/foreign_calls/rpc.rs @@ -0,0 +1,225 @@ +use std::path::PathBuf; + +use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField}; +use jsonrpc::{arg as build_json_rpc_arg, minreq_http::Builder, Client}; +use noirc_printable_type::ForeignCallError; +use serde::{Deserialize, Serialize}; + +use super::ForeignCallExecutor; + +#[derive(Debug)] +pub(super) struct RPCForeignCallExecutor { + /// A randomly generated id for this `DefaultForeignCallExecutor`. + /// + /// This is used so that a single `external_resolver` can distinguish between requests from multiple + /// instantiations of `DefaultForeignCallExecutor`. + id: u64, + /// JSON RPC client to resolve foreign calls + external_resolver: Client, + /// Root path to the program or workspace in execution. + root_path: Option, + /// Name of the package in execution + package_name: Option, +} + +#[derive(Debug, Serialize, Deserialize)] +struct ResolveForeignCallRequest { + /// A session ID which allows the external RPC server to link this foreign call request to other foreign calls + /// for the same program execution. + /// + /// This is intended to allow a single RPC server to maintain state related to multiple program executions being + /// performed in parallel. + session_id: u64, + + #[serde(flatten)] + /// The foreign call which the external RPC server is to provide a response for. + function_call: ForeignCallWaitInfo, + + #[serde(skip_serializing_if = "Option::is_none")] + /// Root path to the program or workspace in execution. + root_path: Option, + #[serde(skip_serializing_if = "Option::is_none")] + /// Name of the package in execution + package_name: Option, +} + +impl RPCForeignCallExecutor { + pub(super) fn new( + resolver_url: &str, + id: u64, + root_path: Option, + package_name: Option, + ) -> Self { + let mut transport_builder = + Builder::new().url(resolver_url).expect("Invalid oracle resolver URL"); + + if let Some(Ok(timeout)) = + std::env::var("NARGO_FOREIGN_CALL_TIMEOUT").ok().map(|timeout| timeout.parse()) + { + let timeout_duration = std::time::Duration::from_millis(timeout); + transport_builder = transport_builder.timeout(timeout_duration); + }; + let oracle_resolver = Client::with_transport(transport_builder.build()); + + RPCForeignCallExecutor { external_resolver: oracle_resolver, id, root_path, package_name } + } +} + +impl Deserialize<'a>> ForeignCallExecutor + for RPCForeignCallExecutor +{ + fn execute( + &mut self, + foreign_call: &ForeignCallWaitInfo, + ) -> Result, ForeignCallError> { + let encoded_params = vec![build_json_rpc_arg(ResolveForeignCallRequest { + session_id: self.id, + function_call: foreign_call.clone(), + root_path: self.root_path.clone().map(|path| path.to_str().unwrap().to_string()), + package_name: self.package_name.clone(), + })]; + + let req = self.external_resolver.build_request("resolve_foreign_call", &encoded_params); + + let response = self.external_resolver.send_request(req)?; + + let parsed_response: ForeignCallResult = response.result()?; + + Ok(parsed_response) + } +} + +#[cfg(test)] +mod tests { + use acvm::{ + acir::brillig::ForeignCallParam, brillig_vm::brillig::ForeignCallResult, + pwg::ForeignCallWaitInfo, FieldElement, + }; + use jsonrpc_core::Result as RpcResult; + use jsonrpc_derive::rpc; + use jsonrpc_http_server::{Server, ServerBuilder}; + + use super::{ForeignCallExecutor, RPCForeignCallExecutor, ResolveForeignCallRequest}; + + #[allow(unreachable_pub)] + #[rpc] + pub trait OracleResolver { + #[rpc(name = "resolve_foreign_call")] + fn resolve_foreign_call( + &self, + req: ResolveForeignCallRequest, + ) -> RpcResult>; + } + + struct OracleResolverImpl; + + impl OracleResolverImpl { + fn echo(&self, param: ForeignCallParam) -> ForeignCallResult { + vec![param].into() + } + + fn sum(&self, array: ForeignCallParam) -> ForeignCallResult { + let mut res: FieldElement = 0_usize.into(); + + for value in array.fields() { + res += value; + } + + res.into() + } + } + + impl OracleResolver for OracleResolverImpl { + fn resolve_foreign_call( + &self, + req: ResolveForeignCallRequest, + ) -> RpcResult> { + let response = match req.function_call.function.as_str() { + "sum" => self.sum(req.function_call.inputs[0].clone()), + "echo" => self.echo(req.function_call.inputs[0].clone()), + "id" => FieldElement::from(req.session_id as u128).into(), + + _ => panic!("unexpected foreign call"), + }; + Ok(response) + } + } + + fn build_oracle_server() -> (Server, String) { + let mut io = jsonrpc_core::IoHandler::new(); + io.extend_with(OracleResolverImpl.to_delegate()); + + // Choosing port 0 results in a random port being assigned. + let server = ServerBuilder::new(io) + .start_http(&"127.0.0.1:0".parse().expect("Invalid address")) + .expect("Could not start server"); + + let url = format!("http://{}", server.address()); + (server, url) + } + + #[test] + fn test_oracle_resolver_echo() { + let (server, url) = build_oracle_server(); + + let mut executor = RPCForeignCallExecutor::new(&url, 1, None, None); + + let foreign_call: ForeignCallWaitInfo = ForeignCallWaitInfo { + function: "echo".to_string(), + inputs: vec![ForeignCallParam::Single(1_u128.into())], + }; + + let result = executor.execute(&foreign_call); + assert_eq!(result.unwrap(), ForeignCallResult { values: foreign_call.inputs }); + + server.close(); + } + + #[test] + fn test_oracle_resolver_sum() { + let (server, url) = build_oracle_server(); + + let mut executor = RPCForeignCallExecutor::new(&url, 2, None, None); + + let foreign_call: ForeignCallWaitInfo = ForeignCallWaitInfo { + function: "sum".to_string(), + inputs: vec![ForeignCallParam::Array(vec![1_usize.into(), 2_usize.into()])], + }; + + let result = executor.execute(&foreign_call); + assert_eq!(result.unwrap(), FieldElement::from(3_usize).into()); + + server.close(); + } + + #[test] + fn foreign_call_executor_id_is_persistent() { + let (server, url) = build_oracle_server(); + + let mut executor = RPCForeignCallExecutor::new(&url, 3, None, None); + + let foreign_call: ForeignCallWaitInfo = ForeignCallWaitInfo { function: "id".to_string(), inputs: Vec::new() }; + + let result_1 = executor.execute(&foreign_call).unwrap(); + let result_2 = executor.execute(&foreign_call).unwrap(); + assert_eq!(result_1, result_2); + + server.close(); + } + + #[test] + fn oracle_resolver_rpc_can_distinguish_executors() { + let (server, url) = build_oracle_server(); + + let mut executor_1 = RPCForeignCallExecutor::new(&url, 4, None, None); + let mut executor_2 = RPCForeignCallExecutor::new(&url, 5, None, None); + + let foreign_call: ForeignCallWaitInfo = ForeignCallWaitInfo { function: "id".to_string(), inputs: Vec::new() }; + + let result_1 = executor_1.execute(&foreign_call).unwrap(); + let result_2 = executor_2.execute(&foreign_call).unwrap(); + assert_ne!(result_1, result_2); + + server.close(); + } +} diff --git a/tooling/nargo/src/lib.rs b/tooling/nargo/src/lib.rs index 88f07e0c292..74b7f54d860 100644 --- a/tooling/nargo/src/lib.rs +++ b/tooling/nargo/src/lib.rs @@ -9,6 +9,7 @@ pub mod constants; pub mod errors; +pub mod foreign_calls; pub mod ops; pub mod package; pub mod workspace; diff --git a/tooling/nargo/src/ops/execute.rs b/tooling/nargo/src/ops/execute.rs index 09ef554d2aa..57116ec2efd 100644 --- a/tooling/nargo/src/ops/execute.rs +++ b/tooling/nargo/src/ops/execute.rs @@ -10,10 +10,9 @@ use acvm::{acir::circuit::Circuit, acir::native_types::WitnessMap}; use acvm::{AcirField, BlackBoxFunctionSolver}; use crate::errors::ExecutionError; +use crate::foreign_calls::ForeignCallExecutor; use crate::NargoError; -use super::foreign_calls::ForeignCallExecutor; - struct ProgramExecutor<'a, F, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> { functions: &'a [Circuit], diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs deleted file mode 100644 index 30785949a46..00000000000 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ /dev/null @@ -1,494 +0,0 @@ -use std::path::PathBuf; - -use acvm::{ - acir::brillig::{ForeignCallParam, ForeignCallResult}, - pwg::ForeignCallWaitInfo, - AcirField, -}; -use jsonrpc::{arg as build_json_rpc_arg, minreq_http::Builder, Client}; -use noirc_printable_type::{decode_string_value, ForeignCallError, PrintableValueDisplay}; -use rand::Rng; -use serde::{Deserialize, Serialize}; - -pub trait ForeignCallExecutor { - fn execute( - &mut self, - foreign_call: &ForeignCallWaitInfo, - ) -> Result, ForeignCallError>; -} - -/// This enumeration represents the Brillig foreign calls that are natively supported by nargo. -/// After resolution of a foreign call, nargo will restart execution of the ACVM -pub enum ForeignCall { - Print, - CreateMock, - SetMockParams, - GetMockLastParams, - SetMockReturns, - SetMockTimes, - ClearMock, -} - -impl std::fmt::Display for ForeignCall { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.name()) - } -} - -impl ForeignCall { - pub(crate) fn name(&self) -> &'static str { - match self { - ForeignCall::Print => "print", - ForeignCall::CreateMock => "create_mock", - ForeignCall::SetMockParams => "set_mock_params", - ForeignCall::GetMockLastParams => "get_mock_last_params", - ForeignCall::SetMockReturns => "set_mock_returns", - ForeignCall::SetMockTimes => "set_mock_times", - ForeignCall::ClearMock => "clear_mock", - } - } - - pub(crate) fn lookup(op_name: &str) -> Option { - match op_name { - "print" => Some(ForeignCall::Print), - "create_mock" => Some(ForeignCall::CreateMock), - "set_mock_params" => Some(ForeignCall::SetMockParams), - "get_mock_last_params" => Some(ForeignCall::GetMockLastParams), - "set_mock_returns" => Some(ForeignCall::SetMockReturns), - "set_mock_times" => Some(ForeignCall::SetMockTimes), - "clear_mock" => Some(ForeignCall::ClearMock), - _ => None, - } - } -} - -/// This struct represents an oracle mock. It can be used for testing programs that use oracles. -#[derive(Debug, PartialEq, Eq, Clone)] -struct MockedCall { - /// The id of the mock, used to update or remove it - id: usize, - /// The oracle it's mocking - name: String, - /// Optionally match the parameters - params: Option>>, - /// The parameters with which the mock was last called - last_called_params: Option>>, - /// The result to return when this mock is called - result: ForeignCallResult, - /// How many times should this mock be called before it is removed - times_left: Option, -} - -impl MockedCall { - fn new(id: usize, name: String) -> Self { - Self { - id, - name, - params: None, - last_called_params: None, - result: ForeignCallResult { values: vec![] }, - times_left: None, - } - } -} - -impl MockedCall { - fn matches(&self, name: &str, params: &[ForeignCallParam]) -> bool { - self.name == name && (self.params.is_none() || self.params.as_deref() == Some(params)) - } -} - -#[derive(Debug, Default)] -pub struct DefaultForeignCallExecutor { - /// A randomly generated id for this `DefaultForeignCallExecutor`. - /// - /// This is used so that a single `external_resolver` can distinguish between requests from multiple - /// instantiations of `DefaultForeignCallExecutor`. - id: u64, - - /// Mocks have unique ids used to identify them in Noir, allowing to update or remove them. - last_mock_id: usize, - /// The registered mocks - mocked_responses: Vec>, - /// Whether to print [`ForeignCall::Print`] output. - show_output: bool, - /// JSON RPC client to resolve foreign calls - external_resolver: Option, - /// Root path to the program or workspace in execution. - root_path: Option, - /// Name of the package in execution - package_name: Option, -} - -#[derive(Debug, Serialize, Deserialize)] -struct ResolveForeignCallRequest { - /// A session ID which allows the external RPC server to link this foreign call request to other foreign calls - /// for the same program execution. - /// - /// This is intended to allow a single RPC server to maintain state related to multiple program executions being - /// performed in parallel. - session_id: u64, - - #[serde(flatten)] - /// The foreign call which the external RPC server is to provide a response for. - function_call: ForeignCallWaitInfo, - - #[serde(skip_serializing_if = "Option::is_none")] - /// Root path to the program or workspace in execution. - root_path: Option, - #[serde(skip_serializing_if = "Option::is_none")] - /// Name of the package in execution - package_name: Option, -} - -impl DefaultForeignCallExecutor { - pub fn new( - show_output: bool, - resolver_url: Option<&str>, - root_path: Option, - package_name: Option, - ) -> Self { - let oracle_resolver = resolver_url.map(|resolver_url| { - let mut transport_builder = - Builder::new().url(resolver_url).expect("Invalid oracle resolver URL"); - - if let Some(Ok(timeout)) = - std::env::var("NARGO_FOREIGN_CALL_TIMEOUT").ok().map(|timeout| timeout.parse()) - { - let timeout_duration = std::time::Duration::from_millis(timeout); - transport_builder = transport_builder.timeout(timeout_duration); - }; - Client::with_transport(transport_builder.build()) - }); - DefaultForeignCallExecutor { - show_output, - external_resolver: oracle_resolver, - id: rand::thread_rng().gen(), - mocked_responses: Vec::new(), - last_mock_id: 0, - root_path, - package_name, - } - } -} - -impl DefaultForeignCallExecutor { - fn extract_mock_id( - foreign_call_inputs: &[ForeignCallParam], - ) -> Result<(usize, &[ForeignCallParam]), ForeignCallError> { - let (id, params) = - foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; - let id = - usize::try_from(id.unwrap_field().try_to_u64().expect("value does not fit into u64")) - .expect("value does not fit into usize"); - Ok((id, params)) - } - - fn find_mock_by_id(&self, id: usize) -> Option<&MockedCall> { - self.mocked_responses.iter().find(|response| response.id == id) - } - - fn find_mock_by_id_mut(&mut self, id: usize) -> Option<&mut MockedCall> { - self.mocked_responses.iter_mut().find(|response| response.id == id) - } - - fn parse_string(param: &ForeignCallParam) -> String { - let fields: Vec<_> = param.fields().to_vec(); - decode_string_value(&fields) - } - - fn execute_print(foreign_call_inputs: &[ForeignCallParam]) -> Result<(), ForeignCallError> { - let skip_newline = foreign_call_inputs[0].unwrap_field().is_zero(); - - let foreign_call_inputs = - foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?.1; - let display_string = Self::format_printable_value(foreign_call_inputs, skip_newline)?; - - print!("{display_string}"); - - Ok(()) - } - - fn format_printable_value( - foreign_call_inputs: &[ForeignCallParam], - skip_newline: bool, - ) -> Result { - let display_values: PrintableValueDisplay = foreign_call_inputs.try_into()?; - - let result = format!("{display_values}{}", if skip_newline { "" } else { "\n" }); - - Ok(result) - } -} - -impl Deserialize<'a>> ForeignCallExecutor - for DefaultForeignCallExecutor -{ - fn execute( - &mut self, - foreign_call: &ForeignCallWaitInfo, - ) -> Result, ForeignCallError> { - let foreign_call_name = foreign_call.function.as_str(); - match ForeignCall::lookup(foreign_call_name) { - Some(ForeignCall::Print) => { - if self.show_output { - Self::execute_print(&foreign_call.inputs)?; - } - Ok(ForeignCallResult::default()) - } - Some(ForeignCall::CreateMock) => { - let mock_oracle_name = Self::parse_string(&foreign_call.inputs[0]); - assert!(ForeignCall::lookup(&mock_oracle_name).is_none()); - let id = self.last_mock_id; - self.mocked_responses.push(MockedCall::new(id, mock_oracle_name)); - self.last_mock_id += 1; - - Ok(F::from(id).into()) - } - Some(ForeignCall::SetMockParams) => { - let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; - self.find_mock_by_id_mut(id) - .unwrap_or_else(|| panic!("Unknown mock id {}", id)) - .params = Some(params.to_vec()); - - Ok(ForeignCallResult::default()) - } - Some(ForeignCall::GetMockLastParams) => { - let (id, _) = Self::extract_mock_id(&foreign_call.inputs)?; - let mock = - self.find_mock_by_id(id).unwrap_or_else(|| panic!("Unknown mock id {}", id)); - - let last_called_params = mock - .last_called_params - .clone() - .unwrap_or_else(|| panic!("Mock {} was never called", mock.name)); - - Ok(last_called_params.into()) - } - Some(ForeignCall::SetMockReturns) => { - let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; - self.find_mock_by_id_mut(id) - .unwrap_or_else(|| panic!("Unknown mock id {}", id)) - .result = ForeignCallResult { values: params.to_vec() }; - - Ok(ForeignCallResult::default()) - } - Some(ForeignCall::SetMockTimes) => { - let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; - let times = - params[0].unwrap_field().try_to_u64().expect("Invalid bit size of times"); - - self.find_mock_by_id_mut(id) - .unwrap_or_else(|| panic!("Unknown mock id {}", id)) - .times_left = Some(times); - - Ok(ForeignCallResult::default()) - } - Some(ForeignCall::ClearMock) => { - let (id, _) = Self::extract_mock_id(&foreign_call.inputs)?; - self.mocked_responses.retain(|response| response.id != id); - Ok(ForeignCallResult::default()) - } - None => { - let mock_response_position = self - .mocked_responses - .iter() - .position(|response| response.matches(foreign_call_name, &foreign_call.inputs)); - - if let Some(response_position) = mock_response_position { - // If the program has registered a mocked response to this oracle call then we prefer responding - // with that. - - let mock = self - .mocked_responses - .get_mut(response_position) - .expect("Invalid position of mocked response"); - - mock.last_called_params = Some(foreign_call.inputs.clone()); - - let result = mock.result.values.clone(); - - if let Some(times_left) = &mut mock.times_left { - *times_left -= 1; - if *times_left == 0 { - self.mocked_responses.remove(response_position); - } - } - - Ok(result.into()) - } else if let Some(external_resolver) = &self.external_resolver { - // If the user has registered an external resolver then we forward any remaining oracle calls there. - - let encoded_params = vec![build_json_rpc_arg(ResolveForeignCallRequest { - session_id: self.id, - function_call: foreign_call.clone(), - root_path: self - .root_path - .clone() - .map(|path| path.to_str().unwrap().to_string()), - package_name: self.package_name.clone(), - })]; - - let req = - external_resolver.build_request("resolve_foreign_call", &encoded_params); - - let response = external_resolver.send_request(req)?; - - let parsed_response: ForeignCallResult = response.result()?; - - Ok(parsed_response) - } else { - // If there's no registered mock oracle response and no registered resolver then we cannot - // return a correct response to the ACVM. The best we can do is to return an empty response, - // this allows us to ignore any foreign calls which exist solely to pass information from inside - // the circuit to the environment (e.g. custom logging) as the execution will still be able to progress. - // - // We optimistically return an empty response for all oracle calls as the ACVM will error - // should a response have been required. - Ok(ForeignCallResult::default()) - } - } - } - } -} - -#[cfg(test)] -mod tests { - use acvm::{ - acir::brillig::ForeignCallParam, brillig_vm::brillig::ForeignCallResult, - pwg::ForeignCallWaitInfo, FieldElement, - }; - use jsonrpc_core::Result as RpcResult; - use jsonrpc_derive::rpc; - use jsonrpc_http_server::{Server, ServerBuilder}; - - use crate::ops::{DefaultForeignCallExecutor, ForeignCallExecutor}; - - use super::ResolveForeignCallRequest; - - #[allow(unreachable_pub)] - #[rpc] - pub trait OracleResolver { - #[rpc(name = "resolve_foreign_call")] - fn resolve_foreign_call( - &self, - req: ResolveForeignCallRequest, - ) -> RpcResult>; - } - - struct OracleResolverImpl; - - impl OracleResolverImpl { - fn echo(&self, param: ForeignCallParam) -> ForeignCallResult { - vec![param].into() - } - - fn sum(&self, array: ForeignCallParam) -> ForeignCallResult { - let mut res: FieldElement = 0_usize.into(); - - for value in array.fields() { - res += value; - } - - res.into() - } - } - - impl OracleResolver for OracleResolverImpl { - fn resolve_foreign_call( - &self, - req: ResolveForeignCallRequest, - ) -> RpcResult> { - let response = match req.function_call.function.as_str() { - "sum" => self.sum(req.function_call.inputs[0].clone()), - "echo" => self.echo(req.function_call.inputs[0].clone()), - "id" => FieldElement::from(req.session_id as u128).into(), - - _ => panic!("unexpected foreign call"), - }; - Ok(response) - } - } - - fn build_oracle_server() -> (Server, String) { - let mut io = jsonrpc_core::IoHandler::new(); - io.extend_with(OracleResolverImpl.to_delegate()); - - // Choosing port 0 results in a random port being assigned. - let server = ServerBuilder::new(io) - .start_http(&"127.0.0.1:0".parse().expect("Invalid address")) - .expect("Could not start server"); - - let url = format!("http://{}", server.address()); - (server, url) - } - - #[test] - fn test_oracle_resolver_echo() { - let (server, url) = build_oracle_server(); - - let mut executor = - DefaultForeignCallExecutor::::new(false, Some(&url), None, None); - - let foreign_call = ForeignCallWaitInfo { - function: "echo".to_string(), - inputs: vec![ForeignCallParam::Single(1_u128.into())], - }; - - let result = executor.execute(&foreign_call); - assert_eq!(result.unwrap(), ForeignCallResult { values: foreign_call.inputs }); - - server.close(); - } - - #[test] - fn test_oracle_resolver_sum() { - let (server, url) = build_oracle_server(); - - let mut executor = DefaultForeignCallExecutor::new(false, Some(&url), None, None); - - let foreign_call = ForeignCallWaitInfo { - function: "sum".to_string(), - inputs: vec![ForeignCallParam::Array(vec![1_usize.into(), 2_usize.into()])], - }; - - let result = executor.execute(&foreign_call); - assert_eq!(result.unwrap(), FieldElement::from(3_usize).into()); - - server.close(); - } - - #[test] - fn foreign_call_executor_id_is_persistent() { - let (server, url) = build_oracle_server(); - - let mut executor = - DefaultForeignCallExecutor::::new(false, Some(&url), None, None); - - let foreign_call = ForeignCallWaitInfo { function: "id".to_string(), inputs: Vec::new() }; - - let result_1 = executor.execute(&foreign_call).unwrap(); - let result_2 = executor.execute(&foreign_call).unwrap(); - assert_eq!(result_1, result_2); - - server.close(); - } - - #[test] - fn oracle_resolver_rpc_can_distinguish_executors() { - let (server, url) = build_oracle_server(); - - let mut executor_1 = - DefaultForeignCallExecutor::::new(false, Some(&url), None, None); - let mut executor_2 = - DefaultForeignCallExecutor::::new(false, Some(&url), None, None); - - let foreign_call = ForeignCallWaitInfo { function: "id".to_string(), inputs: Vec::new() }; - - let result_1 = executor_1.execute(&foreign_call).unwrap(); - let result_2 = executor_2.execute(&foreign_call).unwrap(); - assert_ne!(result_1, result_2); - - server.close(); - } -} diff --git a/tooling/nargo/src/ops/mod.rs b/tooling/nargo/src/ops/mod.rs index f70577a14f1..04efeb5a9ec 100644 --- a/tooling/nargo/src/ops/mod.rs +++ b/tooling/nargo/src/ops/mod.rs @@ -4,7 +4,6 @@ pub use self::compile::{ compile_workspace, report_errors, }; pub use self::execute::{execute_program, execute_program_with_profiling}; -pub use self::foreign_calls::{DefaultForeignCallExecutor, ForeignCall, ForeignCallExecutor}; pub use self::optimize::{optimize_contract, optimize_program}; pub use self::transform::{transform_contract, transform_program}; @@ -13,7 +12,6 @@ pub use self::test::{run_test, TestStatus}; mod check; mod compile; mod execute; -mod foreign_calls; mod optimize; mod test; mod transform; diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 370a4235f61..51d7b8c55aa 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -9,9 +9,11 @@ use noirc_driver::{compile_no_check, CompileError, CompileOptions}; use noirc_errors::{debug_info::DebugInfo, FileDiagnostic}; use noirc_frontend::hir::{def_map::TestFunction, Context}; -use crate::{errors::try_to_diagnose_runtime_error, NargoError}; +use crate::{ + errors::try_to_diagnose_runtime_error, foreign_calls::DefaultForeignCallExecutor, NargoError, +}; -use super::{execute_program, DefaultForeignCallExecutor}; +use super::execute_program; pub enum TestStatus { Pass, diff --git a/tooling/nargo_cli/benches/criterion.rs b/tooling/nargo_cli/benches/criterion.rs index 488cbfcd243..51de97df139 100644 --- a/tooling/nargo_cli/benches/criterion.rs +++ b/tooling/nargo_cli/benches/criterion.rs @@ -115,7 +115,7 @@ fn criterion_test_execution(c: &mut Criterion, test_program_dir: &Path, force_br let artifacts = RefCell::new(None); let mut foreign_call_executor = - nargo::ops::DefaultForeignCallExecutor::new(false, None, None, None); + nargo::foreign_calls::DefaultForeignCallExecutor::new(false, None, None, None); c.bench_function(&benchmark_name, |b| { b.iter_batched( diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 8dc71b1c7e5..fa95d3123c6 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::ops::DefaultForeignCallExecutor; +use nargo::foreign_calls::DefaultForeignCallExecutor; use nargo::package::{CrateName, Package}; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_abi::input_parser::{Format, InputValue}; diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index cf416b1fa5f..769a1f79d81 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -4,7 +4,7 @@ use clap::Args; use iter_extended::vecmap; use nargo::{ constants::PROVER_INPUT_FILE, - ops::DefaultForeignCallExecutor, + foreign_calls::DefaultForeignCallExecutor, package::{CrateName, Package}, }; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; diff --git a/tooling/nargo_cli/tests/stdlib-props.rs b/tooling/nargo_cli/tests/stdlib-props.rs index 0013a90b4ff..12d14bb305d 100644 --- a/tooling/nargo_cli/tests/stdlib-props.rs +++ b/tooling/nargo_cli/tests/stdlib-props.rs @@ -3,7 +3,8 @@ use std::{cell::RefCell, collections::BTreeMap, path::Path}; use acvm::{acir::native_types::WitnessStack, AcirField, FieldElement}; use iter_extended::vecmap; use nargo::{ - ops::{execute_program, DefaultForeignCallExecutor}, + ops::execute_program, + foreign_calls::DefaultForeignCallExecutor, parse_all, }; use noirc_abi::input_parser::InputValue; diff --git a/tooling/profiler/src/cli/execution_flamegraph_cmd.rs b/tooling/profiler/src/cli/execution_flamegraph_cmd.rs index 981d08a3eb1..6d6da89f660 100644 --- a/tooling/profiler/src/cli/execution_flamegraph_cmd.rs +++ b/tooling/profiler/src/cli/execution_flamegraph_cmd.rs @@ -8,7 +8,7 @@ use crate::flamegraph::{BrilligExecutionSample, FlamegraphGenerator, InfernoFlam use crate::fs::{read_inputs_from_file, read_program_from_file}; use crate::opcode_formatter::format_brillig_opcode; use bn254_blackbox_solver::Bn254BlackBoxSolver; -use nargo::ops::DefaultForeignCallExecutor; +use nargo::foreign_calls::DefaultForeignCallExecutor; use noirc_abi::input_parser::Format; use noirc_artifacts::debug::DebugArtifact; From 408df3e52853caed5be8b8ece2756c4801bee46d Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 29 Nov 2024 19:28:52 +0000 Subject: [PATCH 2/5] . --- tooling/nargo/src/foreign_calls/mod.rs | 10 ++++------ tooling/nargo/src/foreign_calls/rpc.rs | 6 ++++-- tooling/nargo_cli/tests/stdlib-props.rs | 6 +----- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/tooling/nargo/src/foreign_calls/mod.rs b/tooling/nargo/src/foreign_calls/mod.rs index 5e14757e24e..0ef3247ee59 100644 --- a/tooling/nargo/src/foreign_calls/mod.rs +++ b/tooling/nargo/src/foreign_calls/mod.rs @@ -85,7 +85,6 @@ impl DefaultForeignCallExecutor { RPCForeignCallExecutor::new(resolver_url, id, root_path, package_name) }); DefaultForeignCallExecutor { - printer, mocker: MockForeignCallExecutor::default(), external: external_resolver, @@ -122,17 +121,17 @@ impl Deserialize<'a>> ForeignCallExecutor // First check if there's any defined mock responses for this foreign call. match self.mocker.execute(foreign_call) { Err(ForeignCallError::NoHandler(_)) => (), - response_or_error => return response_or_error + response_or_error => return response_or_error, }; if let Some(external_resolver) = &mut self.external { // If the user has registered an external resolver then we forward any remaining oracle calls there. match external_resolver.execute(foreign_call) { Err(ForeignCallError::NoHandler(_)) => (), - response_or_error => return response_or_error + response_or_error => return response_or_error, }; - } - + } + // If all executors have no handler for the given foreign call then we cannot // return a correct response to the ACVM. The best we can do is to return an empty response, // this allows us to ignore any foreign calls which exist solely to pass information from inside @@ -141,7 +140,6 @@ impl Deserialize<'a>> ForeignCallExecutor // We optimistically return an empty response for all oracle calls as the ACVM will error // should a response have been required. Ok(ForeignCallResult::default()) - } } } diff --git a/tooling/nargo/src/foreign_calls/rpc.rs b/tooling/nargo/src/foreign_calls/rpc.rs index b9ee7119d6b..dab64819b56 100644 --- a/tooling/nargo/src/foreign_calls/rpc.rs +++ b/tooling/nargo/src/foreign_calls/rpc.rs @@ -198,7 +198,8 @@ mod tests { let mut executor = RPCForeignCallExecutor::new(&url, 3, None, None); - let foreign_call: ForeignCallWaitInfo = ForeignCallWaitInfo { function: "id".to_string(), inputs: Vec::new() }; + let foreign_call: ForeignCallWaitInfo = + ForeignCallWaitInfo { function: "id".to_string(), inputs: Vec::new() }; let result_1 = executor.execute(&foreign_call).unwrap(); let result_2 = executor.execute(&foreign_call).unwrap(); @@ -214,7 +215,8 @@ mod tests { let mut executor_1 = RPCForeignCallExecutor::new(&url, 4, None, None); let mut executor_2 = RPCForeignCallExecutor::new(&url, 5, None, None); - let foreign_call: ForeignCallWaitInfo = ForeignCallWaitInfo { function: "id".to_string(), inputs: Vec::new() }; + let foreign_call: ForeignCallWaitInfo = + ForeignCallWaitInfo { function: "id".to_string(), inputs: Vec::new() }; let result_1 = executor_1.execute(&foreign_call).unwrap(); let result_2 = executor_2.execute(&foreign_call).unwrap(); diff --git a/tooling/nargo_cli/tests/stdlib-props.rs b/tooling/nargo_cli/tests/stdlib-props.rs index 12d14bb305d..370d2c04052 100644 --- a/tooling/nargo_cli/tests/stdlib-props.rs +++ b/tooling/nargo_cli/tests/stdlib-props.rs @@ -2,11 +2,7 @@ use std::{cell::RefCell, collections::BTreeMap, path::Path}; use acvm::{acir::native_types::WitnessStack, AcirField, FieldElement}; use iter_extended::vecmap; -use nargo::{ - ops::execute_program, - foreign_calls::DefaultForeignCallExecutor, - parse_all, -}; +use nargo::{foreign_calls::DefaultForeignCallExecutor, ops::execute_program, parse_all}; use noirc_abi::input_parser::InputValue; use noirc_driver::{ compile_main, file_manager_with_stdlib, prepare_crate, CompilationResult, CompileOptions, From fadd2133107bc3efda74b0e54cdbb4e39bca46c0 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 29 Nov 2024 23:29:39 +0000 Subject: [PATCH 3/5] chore: remove foreign call logic from `noirc_printable_type` --- Cargo.lock | 5 +- compiler/noirc_printable_type/Cargo.toml | 4 - compiler/noirc_printable_type/src/lib.rs | 243 ++++------------------ tooling/debugger/src/foreign_calls.rs | 3 +- tooling/nargo/Cargo.toml | 1 + tooling/nargo/src/errors.rs | 3 +- tooling/nargo/src/foreign_calls/mocker.rs | 5 +- tooling/nargo/src/foreign_calls/mod.rs | 20 +- tooling/nargo/src/foreign_calls/print.rs | 82 ++++++-- tooling/nargo/src/foreign_calls/rpc.rs | 3 +- tooling/noirc_abi/Cargo.toml | 2 +- tooling/noirc_abi/src/lib.rs | 17 +- tooling/noirc_abi/src/printable_type.rs | 78 +++++++ tooling/noirc_artifacts/src/debug_vars.rs | 30 +-- 14 files changed, 237 insertions(+), 259 deletions(-) create mode 100644 tooling/noirc_abi/src/printable_type.rs diff --git a/Cargo.lock b/Cargo.lock index 94a84b89d05..cd05f135ef8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2763,6 +2763,7 @@ dependencies = [ "rand 0.8.5", "rayon", "serde", + "serde_json", "thiserror", "tracing", "walkdir", @@ -3192,12 +3193,8 @@ name = "noirc_printable_type" version = "1.0.0-beta.0" dependencies = [ "acvm", - "iter-extended", - "jsonrpc", "regex", "serde", - "serde_json", - "thiserror", ] [[package]] diff --git a/compiler/noirc_printable_type/Cargo.toml b/compiler/noirc_printable_type/Cargo.toml index 8bb56703e8a..e011d641f8c 100644 --- a/compiler/noirc_printable_type/Cargo.toml +++ b/compiler/noirc_printable_type/Cargo.toml @@ -13,11 +13,7 @@ workspace = true [dependencies] acvm.workspace = true -iter-extended.workspace = true regex = "1.9.1" serde.workspace = true -serde_json.workspace = true -thiserror.workspace = true -jsonrpc.workspace = true [dev-dependencies] diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 838a2472125..b690c6d0a6e 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -1,10 +1,13 @@ +#![forbid(unsafe_code)] +#![warn(unused_crate_dependencies, unused_extern_crates)] +#![warn(unreachable_pub)] +#![warn(clippy::semicolon_if_nothing_returned)] + use std::{collections::BTreeMap, str}; -use acvm::{acir::AcirField, brillig_vm::brillig::ForeignCallParam}; -use iter_extended::vecmap; +use acvm::AcirField; use regex::{Captures, Regex}; use serde::{Deserialize, Serialize}; -use thiserror::Error; #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(tag = "kind", rename_all = "lowercase")] @@ -67,95 +70,28 @@ pub enum PrintableValueDisplay { FmtString(String, Vec<(PrintableValue, PrintableType)>), } -#[derive(Debug, Error)] -pub enum ForeignCallError { - #[error("No handler could be found for foreign call `{0}`")] - NoHandler(String), - - #[error("Foreign call inputs needed for execution are missing")] - MissingForeignCallInputs, - - #[error("Could not parse PrintableType argument. {0}")] - ParsingError(#[from] serde_json::Error), - - #[error("Failed calling external resolver. {0}")] - ExternalResolverError(#[from] jsonrpc::Error), - - #[error("Assert message resolved after an unsatisified constrain. {0}")] - ResolvedAssertMessage(String), -} - -impl TryFrom<&[ForeignCallParam]> for PrintableValueDisplay { - type Error = ForeignCallError; +impl std::fmt::Display for PrintableValueDisplay { + fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Plain(value, typ) => { + let output_string = to_string(value, typ).ok_or(std::fmt::Error)?; + write!(fmt, "{output_string}") + } + Self::FmtString(template, values) => { + let mut display_iter = values.iter(); + let re = Regex::new(r"\{([a-zA-Z0-9_]+)\}").map_err(|_| std::fmt::Error)?; - fn try_from(foreign_call_inputs: &[ForeignCallParam]) -> Result { - let (is_fmt_str, foreign_call_inputs) = - foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?; + let formatted_str = replace_all(&re, template, |_: &Captures| { + let (value, typ) = display_iter.next().ok_or(std::fmt::Error)?; + to_string(value, typ).ok_or(std::fmt::Error) + })?; - if is_fmt_str.unwrap_field().is_one() { - convert_fmt_string_inputs(foreign_call_inputs) - } else { - convert_string_inputs(foreign_call_inputs) + write!(fmt, "{formatted_str}") + } } } } -fn convert_string_inputs( - foreign_call_inputs: &[ForeignCallParam], -) -> Result, ForeignCallError> { - // Fetch the PrintableType from the foreign call input - // The remaining input values should hold what is to be printed - let (printable_type_as_values, input_values) = - foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?; - let printable_type = fetch_printable_type(printable_type_as_values)?; - - // We must use a flat map here as each value in a struct will be in a separate input value - let mut input_values_as_fields = input_values.iter().flat_map(|param| param.fields()); - - let value = decode_value(&mut input_values_as_fields, &printable_type); - - Ok(PrintableValueDisplay::Plain(value, printable_type)) -} - -fn convert_fmt_string_inputs( - foreign_call_inputs: &[ForeignCallParam], -) -> Result, ForeignCallError> { - let (message, input_and_printable_types) = - foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; - - let message_as_fields = message.fields(); - let message_as_string = decode_string_value(&message_as_fields); - - let (num_values, input_and_printable_types) = input_and_printable_types - .split_first() - .ok_or(ForeignCallError::MissingForeignCallInputs)?; - - let mut output = Vec::new(); - let num_values = num_values.unwrap_field().to_u128() as usize; - - let types_start_at = input_and_printable_types.len() - num_values; - let mut input_iter = - input_and_printable_types[0..types_start_at].iter().flat_map(|param| param.fields()); - for printable_type in input_and_printable_types.iter().skip(types_start_at) { - let printable_type = fetch_printable_type(printable_type)?; - let value = decode_value(&mut input_iter, &printable_type); - - output.push((value, printable_type)); - } - - Ok(PrintableValueDisplay::FmtString(message_as_string, output)) -} - -fn fetch_printable_type( - printable_type: &ForeignCallParam, -) -> Result { - let printable_type_as_fields = printable_type.fields(); - let printable_type_as_string = decode_string_value(&printable_type_as_fields); - let printable_type: PrintableType = serde_json::from_str(&printable_type_as_string)?; - - Ok(printable_type) -} - fn to_string(value: &PrintableValue, typ: &PrintableType) -> Option { let mut output = String::new(); match (value, typ) { @@ -253,46 +189,6 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Op Some(output) } -// Taken from Regex docs directly -fn replace_all( - re: &Regex, - haystack: &str, - mut replacement: impl FnMut(&Captures) -> Result, -) -> Result { - let mut new = String::with_capacity(haystack.len()); - let mut last_match = 0; - for caps in re.captures_iter(haystack) { - let m = caps.get(0).unwrap(); - new.push_str(&haystack[last_match..m.start()]); - new.push_str(&replacement(&caps)?); - last_match = m.end(); - } - new.push_str(&haystack[last_match..]); - Ok(new) -} - -impl std::fmt::Display for PrintableValueDisplay { - fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Plain(value, typ) => { - let output_string = to_string(value, typ).ok_or(std::fmt::Error)?; - write!(fmt, "{output_string}") - } - Self::FmtString(template, values) => { - let mut display_iter = values.iter(); - let re = Regex::new(r"\{([a-zA-Z0-9_]+)\}").map_err(|_| std::fmt::Error)?; - - let formatted_str = replace_all(&re, template, |_: &Captures| { - let (value, typ) = display_iter.next().ok_or(std::fmt::Error)?; - to_string(value, typ).ok_or(std::fmt::Error) - })?; - - write!(fmt, "{formatted_str}") - } - } - } -} - /// This trims any leading zeroes. /// A singular '0' will be prepended as well if the trimmed string has an odd length. /// A hex string's length needs to be even to decode into bytes, as two digits correspond to @@ -308,85 +204,20 @@ fn format_field_string(field: F) -> String { "0x".to_owned() + &trimmed_field } -/// Assumes that `field_iterator` contains enough field elements in order to decode the [PrintableType] -pub fn decode_value( - field_iterator: &mut impl Iterator, - typ: &PrintableType, -) -> PrintableValue { - match typ { - PrintableType::Field - | PrintableType::SignedInteger { .. } - | PrintableType::UnsignedInteger { .. } - | PrintableType::Boolean => { - let field_element = field_iterator.next().unwrap(); - - PrintableValue::Field(field_element) - } - PrintableType::Array { length, typ } => { - let length = *length as usize; - let mut array_elements = Vec::with_capacity(length); - for _ in 0..length { - array_elements.push(decode_value(field_iterator, typ)); - } - - PrintableValue::Vec { array_elements, is_slice: false } - } - PrintableType::Slice { typ } => { - let length = field_iterator - .next() - .expect("not enough data to decode variable array length") - .to_u128() as usize; - let mut array_elements = Vec::with_capacity(length); - for _ in 0..length { - array_elements.push(decode_value(field_iterator, typ)); - } - - PrintableValue::Vec { array_elements, is_slice: true } - } - PrintableType::Tuple { types } => PrintableValue::Vec { - array_elements: vecmap(types, |typ| decode_value(field_iterator, typ)), - is_slice: false, - }, - PrintableType::String { length } => { - let field_elements: Vec = field_iterator.take(*length as usize).collect(); - - PrintableValue::String(decode_string_value(&field_elements)) - } - PrintableType::Struct { fields, .. } => { - let mut struct_map = BTreeMap::new(); - - for (field_key, param_type) in fields { - let field_value = decode_value(field_iterator, param_type); - - struct_map.insert(field_key.to_owned(), field_value); - } - - PrintableValue::Struct(struct_map) - } - PrintableType::Function { env, .. } => { - let field_element = field_iterator.next().unwrap(); - let func_ref = PrintableValue::Field(field_element); - // we want to consume the fields from the environment, but for now they are not actually printed - decode_value(field_iterator, env); - func_ref - } - PrintableType::MutableReference { typ } => { - // we decode the reference, but it's not really used for printing - decode_value(field_iterator, typ) - } - PrintableType::Unit => PrintableValue::Field(F::zero()), +// Taken from Regex docs directly +fn replace_all( + re: &Regex, + haystack: &str, + mut replacement: impl FnMut(&Captures) -> Result, +) -> Result { + let mut new = String::with_capacity(haystack.len()); + let mut last_match = 0; + for caps in re.captures_iter(haystack) { + let m = caps.get(0).unwrap(); + new.push_str(&haystack[last_match..m.start()]); + new.push_str(&replacement(&caps)?); + last_match = m.end(); } -} - -pub fn decode_string_value(field_elements: &[F]) -> String { - // TODO: Replace with `into` when Char is supported - let string_as_slice = vecmap(field_elements, |e| { - let mut field_as_bytes = e.to_be_bytes(); - let char_byte = field_as_bytes.pop().unwrap(); // A character in a string is represented by a u8, thus we just want the last byte of the element - assert!(field_as_bytes.into_iter().all(|b| b == 0)); // Assert that the rest of the field element's bytes are empty - char_byte - }); - - let final_string = str::from_utf8(&string_as_slice).unwrap(); - final_string.to_owned() + new.push_str(&haystack[last_match..]); + Ok(new) } diff --git a/tooling/debugger/src/foreign_calls.rs b/tooling/debugger/src/foreign_calls.rs index ecf27a22f29..886e8508886 100644 --- a/tooling/debugger/src/foreign_calls.rs +++ b/tooling/debugger/src/foreign_calls.rs @@ -3,10 +3,9 @@ use acvm::{ pwg::ForeignCallWaitInfo, AcirField, FieldElement, }; -use nargo::foreign_calls::{DefaultForeignCallExecutor, ForeignCallExecutor}; +use nargo::foreign_calls::{DefaultForeignCallExecutor, ForeignCallError, ForeignCallExecutor}; use noirc_artifacts::debug::{DebugArtifact, DebugVars, StackFrame}; use noirc_errors::debug_info::{DebugFnId, DebugVarId}; -use noirc_printable_type::ForeignCallError; pub(crate) enum DebugForeignCall { VarAssign, diff --git a/tooling/nargo/Cargo.toml b/tooling/nargo/Cargo.toml index 1dbb9978b0b..a8e0d3ceea6 100644 --- a/tooling/nargo/Cargo.toml +++ b/tooling/nargo/Cargo.toml @@ -27,6 +27,7 @@ rayon.workspace = true jsonrpc.workspace = true rand.workspace = true serde.workspace = true +serde_json.workspace = true walkdir = "2.5.0" [target.'cfg(not(target_arch = "wasm32"))'.dependencies] diff --git a/tooling/nargo/src/errors.rs b/tooling/nargo/src/errors.rs index 5256f28e36c..00c411bf7e4 100644 --- a/tooling/nargo/src/errors.rs +++ b/tooling/nargo/src/errors.rs @@ -16,9 +16,10 @@ use noirc_errors::{ pub use noirc_errors::Location; use noirc_driver::CrateName; -use noirc_printable_type::ForeignCallError; use thiserror::Error; +use crate::foreign_calls::ForeignCallError; + /// Errors covering situations where a package cannot be compiled. #[derive(Debug, Error)] pub enum CompileError { diff --git a/tooling/nargo/src/foreign_calls/mocker.rs b/tooling/nargo/src/foreign_calls/mocker.rs index c93d16bbaf6..ccb94eaa24c 100644 --- a/tooling/nargo/src/foreign_calls/mocker.rs +++ b/tooling/nargo/src/foreign_calls/mocker.rs @@ -3,10 +3,9 @@ use acvm::{ pwg::ForeignCallWaitInfo, AcirField, }; -use noirc_printable_type::{decode_string_value, ForeignCallError}; use serde::{Deserialize, Serialize}; -use super::{ForeignCall, ForeignCallExecutor}; +use super::{ForeignCall, ForeignCallExecutor, ForeignCallError}; /// This struct represents an oracle mock. It can be used for testing programs that use oracles. #[derive(Debug, PartialEq, Eq, Clone)] @@ -74,7 +73,7 @@ impl MockForeignCallExecutor { fn parse_string(param: &ForeignCallParam) -> String { let fields: Vec<_> = param.fields().to_vec(); - decode_string_value(&fields) + noirc_abi::decode_string_value(&fields) } } diff --git a/tooling/nargo/src/foreign_calls/mod.rs b/tooling/nargo/src/foreign_calls/mod.rs index 0ef3247ee59..790f4fe1705 100644 --- a/tooling/nargo/src/foreign_calls/mod.rs +++ b/tooling/nargo/src/foreign_calls/mod.rs @@ -2,11 +2,11 @@ use std::path::PathBuf; use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField}; use mocker::MockForeignCallExecutor; -use noirc_printable_type::ForeignCallError; use print::PrintForeignCallExecutor; use rand::Rng; use rpc::RPCForeignCallExecutor; use serde::{Deserialize, Serialize}; +use thiserror::Error; mod mocker; mod print; @@ -64,6 +64,24 @@ impl ForeignCall { } } +#[derive(Debug, Error)] +pub enum ForeignCallError { + #[error("No handler could be found for foreign call `{0}`")] + NoHandler(String), + + #[error("Foreign call inputs needed for execution are missing")] + MissingForeignCallInputs, + + #[error("Could not parse PrintableType argument. {0}")] + ParsingError(#[from] serde_json::Error), + + #[error("Failed calling external resolver. {0}")] + ExternalResolverError(#[from] jsonrpc::Error), + + #[error("Assert message resolved after an unsatisified constrain. {0}")] + ResolvedAssertMessage(String), +} + #[derive(Debug, Default)] pub struct DefaultForeignCallExecutor { /// The executor for any [`ForeignCall::Print`] calls. diff --git a/tooling/nargo/src/foreign_calls/print.rs b/tooling/nargo/src/foreign_calls/print.rs index d0befa0bd0b..53800ae4d4e 100644 --- a/tooling/nargo/src/foreign_calls/print.rs +++ b/tooling/nargo/src/foreign_calls/print.rs @@ -1,7 +1,9 @@ -use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField}; -use noirc_printable_type::{ForeignCallError, PrintableValueDisplay}; +use acvm::{acir::brillig::{ForeignCallParam, ForeignCallResult}, pwg::ForeignCallWaitInfo, AcirField}; +use noirc_printable_type::{PrintableType, PrintableValueDisplay}; -use super::{ForeignCall, ForeignCallExecutor}; +use noirc_abi::{decode_printable_value as decode_value, decode_string_value}; + +use super::{ForeignCall, ForeignCallError, ForeignCallExecutor}; #[derive(Debug, Default)] pub(super) struct PrintForeignCallExecutor; @@ -15,18 +17,16 @@ impl ForeignCallExecutor for PrintForeignCallExecutor { match ForeignCall::lookup(foreign_call_name) { Some(ForeignCall::Print) => { let skip_newline = foreign_call.inputs[0].unwrap_field().is_zero(); + let is_fmt_str = foreign_call.inputs[1].unwrap_field().is_one(); - let foreign_call_inputs = foreign_call - .inputs - .split_first() - .ok_or(ForeignCallError::MissingForeignCallInputs)? - .1; - - let display_values: PrintableValueDisplay = foreign_call_inputs.try_into()?; - let display_string = - format!("{display_values}{}", if skip_newline { "" } else { "\n" }); + let foreign_call_inputs = &foreign_call.inputs[2..]; + let display_values = if is_fmt_str { + convert_fmt_string_inputs(foreign_call_inputs)? + } else { + convert_string_inputs(foreign_call_inputs)? + }; - print!("{display_string}"); + print!("{display_values}{}", if skip_newline { "" } else { "\n" }); Ok(ForeignCallResult::default()) } @@ -34,3 +34,59 @@ impl ForeignCallExecutor for PrintForeignCallExecutor { } } } + +fn convert_string_inputs( + foreign_call_inputs: &[ForeignCallParam], +) -> Result, ForeignCallError> { + // Fetch the PrintableType from the foreign call input + // The remaining input values should hold what is to be printed + let (printable_type_as_values, input_values) = + foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?; + let printable_type = fetch_printable_type(printable_type_as_values)?; + + // We must use a flat map here as each value in a struct will be in a separate input value + let mut input_values_as_fields = input_values.iter().flat_map(|param| param.fields()); + + let value = decode_value(&mut input_values_as_fields, &printable_type); + + Ok(PrintableValueDisplay::Plain(value, printable_type)) +} + +fn convert_fmt_string_inputs( + foreign_call_inputs: &[ForeignCallParam], +) -> Result, ForeignCallError> { + let (message, input_and_printable_types) = + foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?; + + let message_as_fields = message.fields(); + let message_as_string = decode_string_value(&message_as_fields); + + let (num_values, input_and_printable_types) = input_and_printable_types + .split_first() + .ok_or(ForeignCallError::MissingForeignCallInputs)?; + + let mut output = Vec::new(); + let num_values = num_values.unwrap_field().to_u128() as usize; + + let types_start_at = input_and_printable_types.len() - num_values; + let mut input_iter = + input_and_printable_types[0..types_start_at].iter().flat_map(|param| param.fields()); + for printable_type in input_and_printable_types.iter().skip(types_start_at) { + let printable_type = fetch_printable_type(printable_type)?; + let value = decode_value(&mut input_iter, &printable_type); + + output.push((value, printable_type)); + } + + Ok(PrintableValueDisplay::FmtString(message_as_string, output)) +} + +fn fetch_printable_type( + printable_type: &ForeignCallParam, +) -> Result { + let printable_type_as_fields = printable_type.fields(); + let printable_type_as_string = decode_string_value(&printable_type_as_fields); + let printable_type: PrintableType = serde_json::from_str(&printable_type_as_string)?; + + Ok(printable_type) +} diff --git a/tooling/nargo/src/foreign_calls/rpc.rs b/tooling/nargo/src/foreign_calls/rpc.rs index dab64819b56..9b9732ba49f 100644 --- a/tooling/nargo/src/foreign_calls/rpc.rs +++ b/tooling/nargo/src/foreign_calls/rpc.rs @@ -2,10 +2,9 @@ use std::path::PathBuf; use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField}; use jsonrpc::{arg as build_json_rpc_arg, minreq_http::Builder, Client}; -use noirc_printable_type::ForeignCallError; use serde::{Deserialize, Serialize}; -use super::ForeignCallExecutor; +use super::{ForeignCallExecutor, ForeignCallError}; #[derive(Debug)] pub(super) struct RPCForeignCallExecutor { diff --git a/tooling/noirc_abi/Cargo.toml b/tooling/noirc_abi/Cargo.toml index 22114408e18..79e14471200 100644 --- a/tooling/noirc_abi/Cargo.toml +++ b/tooling/noirc_abi/Cargo.toml @@ -15,10 +15,10 @@ workspace = true acvm.workspace = true iter-extended.workspace = true noirc_printable_type.workspace = true -toml.workspace = true serde_json = "1.0" serde.workspace = true thiserror.workspace = true +toml.workspace = true num-bigint = "0.4" num-traits = "0.2" diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index b1b199727c2..c21b7f6333f 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -13,10 +13,7 @@ use acvm::{ use errors::AbiError; use input_parser::InputValue; use iter_extended::{try_btree_map, try_vecmap, vecmap}; -use noirc_printable_type::{ - decode_value as printable_type_decode_value, PrintableType, PrintableValue, - PrintableValueDisplay, -}; + use serde::{Deserialize, Serialize}; use std::borrow::Borrow; use std::{collections::BTreeMap, str}; @@ -30,8 +27,12 @@ mod arbitrary; pub mod errors; pub mod input_parser; +mod printable_type; mod serialization; +use noirc_printable_type::{PrintableType, PrintableValue, PrintableValueDisplay}; +pub use printable_type::decode_value as decode_printable_value; + /// A map from the fields in an TOML/JSON file which correspond to some ABI to their values pub type InputMap = BTreeMap; @@ -416,7 +417,7 @@ pub fn decode_value( Ok(value) } -fn decode_string_value(field_elements: &[FieldElement]) -> String { +pub fn decode_string_value(field_elements: &[F]) -> String { let string_as_slice = vecmap(field_elements, |e| { let mut field_as_bytes = e.to_be_bytes(); let char_byte = field_as_bytes.pop().unwrap(); // A character in a string is represented by a u8, thus we just want the last byte of the element @@ -475,21 +476,21 @@ pub fn display_abi_error( AbiErrorType::FmtString { length, item_types } => { let mut fields_iter = fields.iter().copied(); let PrintableValue::String(string) = - printable_type_decode_value(&mut fields_iter, &PrintableType::String { length }) + decode_printable_value(&mut fields_iter, &PrintableType::String { length }) else { unreachable!("Got non-string from string decoding"); }; let _length_of_items = fields_iter.next(); let items = item_types.into_iter().map(|abi_type| { let printable_typ = (&abi_type).into(); - let decoded = printable_type_decode_value(&mut fields_iter, &printable_typ); + let decoded = decode_printable_value(&mut fields_iter, &printable_typ); (decoded, printable_typ) }); PrintableValueDisplay::FmtString(string, items.collect()) } AbiErrorType::Custom(abi_typ) => { let printable_type = (&abi_typ).into(); - let decoded = printable_type_decode_value(&mut fields.iter().copied(), &printable_type); + let decoded = decode_printable_value(&mut fields.iter().copied(), &printable_type); PrintableValueDisplay::Plain(decoded, printable_type) } AbiErrorType::String { string } => { diff --git a/tooling/noirc_abi/src/printable_type.rs b/tooling/noirc_abi/src/printable_type.rs new file mode 100644 index 00000000000..a81eb0ce8f6 --- /dev/null +++ b/tooling/noirc_abi/src/printable_type.rs @@ -0,0 +1,78 @@ +use std::collections::BTreeMap; + +use acvm::acir::AcirField; +use iter_extended::vecmap; + +use noirc_printable_type::{PrintableType, PrintableValue}; + +use crate::decode_string_value; + +/// Assumes that `field_iterator` contains enough field elements in order to decode the [PrintableType] +pub fn decode_value( + field_iterator: &mut impl Iterator, + typ: &PrintableType, +) -> PrintableValue { + match typ { + PrintableType::Field + | PrintableType::SignedInteger { .. } + | PrintableType::UnsignedInteger { .. } + | PrintableType::Boolean => { + let field_element = field_iterator.next().unwrap(); + + PrintableValue::Field(field_element) + } + PrintableType::Array { length, typ } => { + let length = *length as usize; + let mut array_elements = Vec::with_capacity(length); + for _ in 0..length { + array_elements.push(decode_value(field_iterator, typ)); + } + + PrintableValue::Vec { array_elements, is_slice: false } + } + PrintableType::Slice { typ } => { + let length = field_iterator + .next() + .expect("not enough data to decode variable array length") + .to_u128() as usize; + let mut array_elements = Vec::with_capacity(length); + for _ in 0..length { + array_elements.push(decode_value(field_iterator, typ)); + } + + PrintableValue::Vec { array_elements, is_slice: true } + } + PrintableType::Tuple { types } => PrintableValue::Vec { + array_elements: vecmap(types, |typ| decode_value(field_iterator, typ)), + is_slice: false, + }, + PrintableType::String { length } => { + let field_elements: Vec = field_iterator.take(*length as usize).collect(); + + PrintableValue::String(decode_string_value(&field_elements)) + } + PrintableType::Struct { fields, .. } => { + let mut struct_map = BTreeMap::new(); + + for (field_key, param_type) in fields { + let field_value = decode_value(field_iterator, param_type); + + struct_map.insert(field_key.to_owned(), field_value); + } + + PrintableValue::Struct(struct_map) + } + PrintableType::Function { env, .. } => { + let field_element = field_iterator.next().unwrap(); + let func_ref = PrintableValue::Field(field_element); + // we want to consume the fields from the environment, but for now they are not actually printed + decode_value(field_iterator, env); + func_ref + } + PrintableType::MutableReference { typ } => { + // we decode the reference, but it's not really used for printing + decode_value(field_iterator, typ) + } + PrintableType::Unit => PrintableValue::Field(F::zero()), + } +} diff --git a/tooling/noirc_artifacts/src/debug_vars.rs b/tooling/noirc_artifacts/src/debug_vars.rs index aa9328432b8..62f06f2c296 100644 --- a/tooling/noirc_artifacts/src/debug_vars.rs +++ b/tooling/noirc_artifacts/src/debug_vars.rs @@ -1,8 +1,10 @@ -use acvm::AcirField; +use acvm::FieldElement; +use noirc_abi::decode_printable_value; use noirc_errors::debug_info::{ DebugFnId, DebugFunction, DebugInfo, DebugTypeId, DebugVarId, DebugVariable, }; -use noirc_printable_type::{decode_value, PrintableType, PrintableValue}; +use noirc_printable_type::{PrintableType, PrintableValue}; + use std::collections::HashMap; #[derive(Debug, Default, Clone)] @@ -19,18 +21,18 @@ pub struct StackFrame<'a, F> { pub variables: Vec<(&'a str, &'a PrintableValue, &'a PrintableType)>, } -impl DebugVars { +impl DebugVars { pub fn insert_debug_info(&mut self, info: &DebugInfo) { self.variables.extend(info.variables.clone()); self.types.extend(info.types.clone()); self.functions.extend(info.functions.clone()); } - pub fn get_variables(&self) -> Vec> { + pub fn get_variables(&self) -> Vec> { self.frames.iter().map(|(fn_id, frame)| self.build_stack_frame(fn_id, frame)).collect() } - pub fn current_stack_frame(&self) -> Option> { + pub fn current_stack_frame(&self) -> Option> { self.frames.last().map(|(fn_id, frame)| self.build_stack_frame(fn_id, frame)) } @@ -44,13 +46,13 @@ impl DebugVars { fn build_stack_frame<'a>( &'a self, fn_id: &DebugFnId, - frame: &'a HashMap>, - ) -> StackFrame { + frame: &'a HashMap>, + ) -> StackFrame { let debug_fn = &self.functions.get(fn_id).expect("failed to find function metadata"); let params: Vec<&str> = debug_fn.arg_names.iter().map(|arg_name| arg_name.as_str()).collect(); - let vars: Vec<(&str, &PrintableValue, &PrintableType)> = frame + let vars: Vec<(&str, &PrintableValue, &PrintableType)> = frame .iter() .filter_map(|(var_id, var_value)| { self.lookup_var(*var_id).map(|(name, typ)| (name, var_value, typ)) @@ -64,7 +66,7 @@ impl DebugVars { } } - pub fn assign_var(&mut self, var_id: DebugVarId, values: &[F]) { + pub fn assign_var(&mut self, var_id: DebugVarId, values: &[FieldElement]) { let type_id = &self.variables.get(&var_id).unwrap().debug_type_id; let ptype = self.types.get(type_id).unwrap(); @@ -72,12 +74,12 @@ impl DebugVars { .last_mut() .expect("unexpected empty stack frames") .1 - .insert(var_id, decode_value(&mut values.iter().copied(), ptype)); + .insert(var_id, decode_printable_value(&mut values.iter().copied(), ptype)); } - pub fn assign_field(&mut self, var_id: DebugVarId, indexes: Vec, values: &[F]) { + pub fn assign_field(&mut self, var_id: DebugVarId, indexes: Vec, values: &[FieldElement]) { let current_frame = &mut self.frames.last_mut().expect("unexpected empty stack frames").1; - let mut cursor: &mut PrintableValue = current_frame + let mut cursor: &mut PrintableValue = current_frame .get_mut(&var_id) .unwrap_or_else(|| panic!("value unavailable for var_id {var_id:?}")); let cursor_type_id = &self @@ -143,10 +145,10 @@ impl DebugVars { } }; } - *cursor = decode_value(&mut values.iter().copied(), cursor_type); + *cursor = decode_printable_value(&mut values.iter().copied(), cursor_type); } - pub fn assign_deref(&mut self, _var_id: DebugVarId, _values: &[F]) { + pub fn assign_deref(&mut self, _var_id: DebugVarId, _values: &[FieldElement]) { unimplemented![] } From 491b9568317b6c6e29766a2585207ca5a98ffa7b Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 29 Nov 2024 23:31:21 +0000 Subject: [PATCH 4/5] . --- tooling/noirc_artifacts/src/debug_vars.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tooling/noirc_artifacts/src/debug_vars.rs b/tooling/noirc_artifacts/src/debug_vars.rs index 62f06f2c296..f8a0e352e5e 100644 --- a/tooling/noirc_artifacts/src/debug_vars.rs +++ b/tooling/noirc_artifacts/src/debug_vars.rs @@ -1,4 +1,4 @@ -use acvm::FieldElement; +use acvm::AcirField; use noirc_abi::decode_printable_value; use noirc_errors::debug_info::{ DebugFnId, DebugFunction, DebugInfo, DebugTypeId, DebugVarId, DebugVariable, @@ -21,18 +21,18 @@ pub struct StackFrame<'a, F> { pub variables: Vec<(&'a str, &'a PrintableValue, &'a PrintableType)>, } -impl DebugVars { +impl DebugVars { pub fn insert_debug_info(&mut self, info: &DebugInfo) { self.variables.extend(info.variables.clone()); self.types.extend(info.types.clone()); self.functions.extend(info.functions.clone()); } - pub fn get_variables(&self) -> Vec> { + pub fn get_variables(&self) -> Vec> { self.frames.iter().map(|(fn_id, frame)| self.build_stack_frame(fn_id, frame)).collect() } - pub fn current_stack_frame(&self) -> Option> { + pub fn current_stack_frame(&self) -> Option> { self.frames.last().map(|(fn_id, frame)| self.build_stack_frame(fn_id, frame)) } @@ -46,13 +46,13 @@ impl DebugVars { fn build_stack_frame<'a>( &'a self, fn_id: &DebugFnId, - frame: &'a HashMap>, - ) -> StackFrame { + frame: &'a HashMap>, + ) -> StackFrame { let debug_fn = &self.functions.get(fn_id).expect("failed to find function metadata"); let params: Vec<&str> = debug_fn.arg_names.iter().map(|arg_name| arg_name.as_str()).collect(); - let vars: Vec<(&str, &PrintableValue, &PrintableType)> = frame + let vars: Vec<(&str, &PrintableValue, &PrintableType)> = frame .iter() .filter_map(|(var_id, var_value)| { self.lookup_var(*var_id).map(|(name, typ)| (name, var_value, typ)) @@ -66,7 +66,7 @@ impl DebugVars { } } - pub fn assign_var(&mut self, var_id: DebugVarId, values: &[FieldElement]) { + pub fn assign_var(&mut self, var_id: DebugVarId, values: &[F]) { let type_id = &self.variables.get(&var_id).unwrap().debug_type_id; let ptype = self.types.get(type_id).unwrap(); @@ -77,9 +77,9 @@ impl DebugVars { .insert(var_id, decode_printable_value(&mut values.iter().copied(), ptype)); } - pub fn assign_field(&mut self, var_id: DebugVarId, indexes: Vec, values: &[FieldElement]) { + pub fn assign_field(&mut self, var_id: DebugVarId, indexes: Vec, values: &[F]) { let current_frame = &mut self.frames.last_mut().expect("unexpected empty stack frames").1; - let mut cursor: &mut PrintableValue = current_frame + let mut cursor: &mut PrintableValue = current_frame .get_mut(&var_id) .unwrap_or_else(|| panic!("value unavailable for var_id {var_id:?}")); let cursor_type_id = &self @@ -148,7 +148,7 @@ impl DebugVars { *cursor = decode_printable_value(&mut values.iter().copied(), cursor_type); } - pub fn assign_deref(&mut self, _var_id: DebugVarId, _values: &[FieldElement]) { + pub fn assign_deref(&mut self, _var_id: DebugVarId, _values: &[F]) { unimplemented![] } From 399db7aac407473332fb5556952d5901dcdcf54c Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Fri, 29 Nov 2024 23:43:13 +0000 Subject: [PATCH 5/5] . --- compiler/noirc_printable_type/src/lib.rs | 2 +- tooling/nargo/src/foreign_calls/mocker.rs | 2 +- tooling/nargo/src/foreign_calls/print.rs | 6 +++++- tooling/nargo/src/foreign_calls/rpc.rs | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index b690c6d0a6e..bde06a7e901 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -129,7 +129,7 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Op (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Array { typ, .. }) | (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Slice { typ }) => { if *is_slice { - output.push('&') + output.push('&'); } output.push('['); let mut values = array_elements.iter().peekable(); diff --git a/tooling/nargo/src/foreign_calls/mocker.rs b/tooling/nargo/src/foreign_calls/mocker.rs index ccb94eaa24c..9fbff649d42 100644 --- a/tooling/nargo/src/foreign_calls/mocker.rs +++ b/tooling/nargo/src/foreign_calls/mocker.rs @@ -5,7 +5,7 @@ use acvm::{ }; use serde::{Deserialize, Serialize}; -use super::{ForeignCall, ForeignCallExecutor, ForeignCallError}; +use super::{ForeignCall, ForeignCallError, ForeignCallExecutor}; /// This struct represents an oracle mock. It can be used for testing programs that use oracles. #[derive(Debug, PartialEq, Eq, Clone)] diff --git a/tooling/nargo/src/foreign_calls/print.rs b/tooling/nargo/src/foreign_calls/print.rs index 53800ae4d4e..0735e9665c6 100644 --- a/tooling/nargo/src/foreign_calls/print.rs +++ b/tooling/nargo/src/foreign_calls/print.rs @@ -1,4 +1,8 @@ -use acvm::{acir::brillig::{ForeignCallParam, ForeignCallResult}, pwg::ForeignCallWaitInfo, AcirField}; +use acvm::{ + acir::brillig::{ForeignCallParam, ForeignCallResult}, + pwg::ForeignCallWaitInfo, + AcirField, +}; use noirc_printable_type::{PrintableType, PrintableValueDisplay}; use noirc_abi::{decode_printable_value as decode_value, decode_string_value}; diff --git a/tooling/nargo/src/foreign_calls/rpc.rs b/tooling/nargo/src/foreign_calls/rpc.rs index 9b9732ba49f..e125036af2e 100644 --- a/tooling/nargo/src/foreign_calls/rpc.rs +++ b/tooling/nargo/src/foreign_calls/rpc.rs @@ -4,7 +4,7 @@ use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField use jsonrpc::{arg as build_json_rpc_arg, minreq_http::Builder, Client}; use serde::{Deserialize, Serialize}; -use super::{ForeignCallExecutor, ForeignCallError}; +use super::{ForeignCallError, ForeignCallExecutor}; #[derive(Debug)] pub(super) struct RPCForeignCallExecutor {