Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Decouple noirc_abi from frontend by introducing PrintableType #2373

Merged
merged 9 commits into from
Aug 23, 2023
18 changes: 15 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ members = [
"crates/noirc_frontend",
"crates/noirc_errors",
"crates/noirc_driver",
"crates/noirc_printable_type",
"crates/nargo",
"crates/nargo_cli",
"crates/nargo_toml",
Expand Down Expand Up @@ -38,6 +39,7 @@ noirc_driver = { path = "crates/noirc_driver" }
noirc_errors = { path = "crates/noirc_errors" }
noirc_evaluator = { path = "crates/noirc_evaluator" }
noirc_frontend = { path = "crates/noirc_frontend" }
noirc_printable_type = { path = "crates/noirc_printable_type" }
noir_wasm = { path = "crates/wasm" }
cfg-if = "1.0.0"
clap = { version = "4.3.19", features = ["derive"] }
Expand Down
3 changes: 1 addition & 2 deletions crates/nargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ fm.workspace = true
noirc_abi.workspace = true
noirc_driver.workspace = true
noirc_frontend.workspace = true
noirc_printable_type.workspace = true
iter-extended.workspace = true
serde.workspace = true
serde_json.workspace = true
thiserror.workspace = true
noirc_errors.workspace = true
base64.workspace = true
regex = "1.9.1"
16 changes: 1 addition & 15 deletions crates/nargo/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use acvm::pwg::OpcodeResolutionError;
use noirc_abi::errors::{AbiError, InputParserError};
use noirc_printable_type::ForeignCallError;
use thiserror::Error;

#[derive(Debug, Error)]
Expand All @@ -15,17 +15,3 @@ pub enum NargoError {
#[error(transparent)]
ForeignCallError(#[from] ForeignCallError),
}

#[derive(Debug, Error)]
pub enum ForeignCallError {
#[error("Foreign call inputs needed for execution are missing")]
MissingForeignCallInputs,

/// ABI encoding/decoding error
#[error(transparent)]
AbiError(#[from] AbiError),

/// Input parsing error
#[error(transparent)]
InputParserError(#[from] InputParserError),
}
94 changes: 6 additions & 88 deletions crates/nargo/src/ops/foreign_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@ use acvm::{
pwg::ForeignCallWaitInfo,
};
use iter_extended::vecmap;
use noirc_abi::{decode_string_value, input_parser::InputValueDisplay, AbiType};
use regex::{Captures, Regex};
use noirc_printable_type::PrintableValueDisplay;

use crate::errors::ForeignCallError;
use crate::NargoError;

/// 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
Expand Down Expand Up @@ -43,7 +42,7 @@ impl ForeignCall {
pub(crate) fn execute(
foreign_call: &ForeignCallWaitInfo,
show_output: bool,
) -> Result<ForeignCallResult, ForeignCallError> {
) -> Result<ForeignCallResult, NargoError> {
let foreign_call_name = foreign_call.function.as_str();
match Self::lookup(foreign_call_name) {
Some(ForeignCall::Println) => {
Expand Down Expand Up @@ -78,90 +77,9 @@ impl ForeignCall {
}
}

fn execute_println(foreign_call_inputs: &[Vec<Value>]) -> Result<(), ForeignCallError> {
let (is_fmt_str, foreign_call_inputs) =
foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?;

let output_string = if is_fmt_str[0].to_field().is_one() {
convert_fmt_string_inputs(foreign_call_inputs)?
} else {
convert_string_inputs(foreign_call_inputs)?
};
println!("{output_string}");
fn execute_println(foreign_call_inputs: &[Vec<Value>]) -> Result<(), NargoError> {
let display_values: PrintableValueDisplay = foreign_call_inputs.try_into()?;
phated marked this conversation as resolved.
Show resolved Hide resolved
println!("{display_values}");
Ok(())
}
}

fn convert_string_inputs(foreign_call_inputs: &[Vec<Value>]) -> Result<String, ForeignCallError> {
// Fetch the abi type from the foreign call input
// The remaining input values should hold what is to be printed
let (abi_type_as_values, input_values) =
foreign_call_inputs.split_last().ok_or(ForeignCallError::MissingForeignCallInputs)?;
let abi_type = fetch_abi_type(abi_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(|values| vecmap(values, |value| value.to_field()));

let input_value_display =
InputValueDisplay::try_from_fields(&mut input_values_as_fields, abi_type)?;

Ok(input_value_display.to_string())
}

fn convert_fmt_string_inputs(
foreign_call_inputs: &[Vec<Value>],
) -> Result<String, ForeignCallError> {
let (message_as_values, input_and_abi_values) =
foreign_call_inputs.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?;

let message_as_fields = vecmap(message_as_values, |value| value.to_field());
let message_as_string = decode_string_value(&message_as_fields);

let (num_values, input_and_abi_values) =
input_and_abi_values.split_first().ok_or(ForeignCallError::MissingForeignCallInputs)?;

let mut output_strings = Vec::new();
let num_values = num_values[0].to_field().to_u128() as usize;

let mut abi_types = Vec::new();
for abi_values in input_and_abi_values.iter().skip(input_and_abi_values.len() - num_values) {
let abi_type = fetch_abi_type(abi_values)?;
abi_types.push(abi_type);
}

for i in 0..num_values {
let abi_type = &abi_types[i];
let type_size = abi_type.field_count() as usize;

let mut input_values_as_fields = input_and_abi_values[i..(i + type_size)]
.iter()
.flat_map(|values| vecmap(values, |value| value.to_field()));

let input_value_display =
InputValueDisplay::try_from_fields(&mut input_values_as_fields, abi_type.clone())?;

output_strings.push(input_value_display.to_string());
}

let mut output_strings_iter = output_strings.into_iter();
let re = Regex::new(r"\{([a-zA-Z0-9_]+)\}")
.expect("ICE: an invalid regex pattern was used for checking format strings");

let formatted_str = re.replace_all(&message_as_string, |_: &Captures| {
output_strings_iter
.next()
.expect("ICE: there are more regex matches than fields supplied to the format string")
});

Ok(formatted_str.into_owned())
}

fn fetch_abi_type(abi_type_as_values: &[Value]) -> Result<AbiType, ForeignCallError> {
let abi_type_as_fields = vecmap(abi_type_as_values, |value| value.to_field());
let abi_type_as_string = decode_string_value(&abi_type_as_fields);
let abi_type: AbiType = serde_json::from_str(&abi_type_as_string)
.map_err(|err| ForeignCallError::InputParserError(err.into()))?;

Ok(abi_type)
}
1 change: 1 addition & 0 deletions crates/noirc_abi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ edition.workspace = true
[dependencies]
acvm.workspace = true
iter-extended.workspace = true
noirc_frontend.workspace = true
toml.workspace = true
serde_json = "1.0"
serde.workspace = true
Expand Down
4 changes: 0 additions & 4 deletions crates/noirc_abi/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ pub enum InputParserError {
ParseStr(String),
#[error("Could not parse hex value {0}")]
ParseHexStr(String),
#[error("duplicate variable name {0}")]
DuplicateVariableName(String),
#[error("cannot parse value into {0:?}")]
AbiTypeMismatch(AbiType),
#[error("Expected argument `{0}`, but none was found")]
Expand All @@ -38,8 +36,6 @@ impl From<serde_json::Error> for InputParserError {

#[derive(Debug, Error)]
pub enum AbiError {
#[error("{0}")]
Generic(String),
#[error("Received parameters not expected by ABI: {0:?}")]
UnexpectedParams(Vec<String>),
#[error("The parameter {} is expected to be a {:?} but found incompatible value {value:?}", .param.name, .param.typ)]
Expand Down
10 changes: 0 additions & 10 deletions crates/noirc_abi/src/input_parser/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,3 @@ impl InputValue {
Ok(input_value)
}
}

impl std::fmt::Display for JsonTypes {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
// From the docs: https://doc.rust-lang.org/std/fmt/struct.Error.html
// This type does not support transmission of an error other than that an error
// occurred. Any extra information must be arranged to be transmitted through
// some other means.
write!(f, "{}", serde_json::to_string(&self).map_err(|_| std::fmt::Error)?)
}
}
33 changes: 2 additions & 31 deletions crates/noirc_abi/src/input_parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use std::collections::BTreeMap;
use acvm::FieldElement;
use serde::Serialize;

use crate::errors::{AbiError, InputParserError};
use crate::{decode_value, Abi, AbiType};
use crate::errors::InputParserError;
use crate::{Abi, AbiType};
/// This is what all formats eventually transform into
/// For example, a toml file will parse into TomlTypes
/// and those TomlTypes will be mapped to Value
Expand Down Expand Up @@ -67,35 +67,6 @@ impl InputValue {
}
}

/// In order to display an `InputValue` we need an `AbiType` to accurately
/// convert the value into a human-readable format.
pub struct InputValueDisplay {
input_value: InputValue,
abi_type: AbiType,
}

impl InputValueDisplay {
pub fn try_from_fields(
field_iterator: &mut impl Iterator<Item = FieldElement>,
abi_type: AbiType,
) -> Result<Self, AbiError> {
let input_value = decode_value(field_iterator, &abi_type)?;
Ok(InputValueDisplay { input_value, abi_type })
}
}

impl std::fmt::Display for InputValueDisplay {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
// From the docs: https://doc.rust-lang.org/std/fmt/struct.Error.html
// This type does not support transmission of an error other than that an error
// occurred. Any extra information must be arranged to be transmitted through
// some other means.
let json_value = json::JsonTypes::try_from_input_value(&self.input_value, &self.abi_type)
.map_err(|_| std::fmt::Error)?;
write!(f, "{}", serde_json::to_string(&json_value).map_err(|_| std::fmt::Error)?)
}
}

/// The different formats that are supported when parsing
/// the initial witness values
#[cfg_attr(test, derive(strum_macros::EnumIter))]
Expand Down
Loading