chore: Decouple noirc_abi from frontend by introducing PrintableType (#2373)

Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>

Author: phated

Cargo.lock

@@ -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",
@@ -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"] }

Cargo.toml

@@ -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"

crates/nargo/Cargo.toml

@@ -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)]
@@ -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),
-}

crates/nargo/src/errors.rs

@@ -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
@@ -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) => {
@@ -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()?;
+        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)
-}

crates/nargo/src/ops/foreign_calls.rs

@@ -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

crates/noirc_abi/Cargo.toml

@@ -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")]
@@ -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)]

crates/noirc_abi/src/errors.rs

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

crates/noirc_abi/src/input_parser/json.rs

@@ -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
@@ -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))]