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
13 changes: 12 additions & 1 deletion 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
1 change: 1 addition & 0 deletions crates/nargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ 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
Expand Down
53 changes: 30 additions & 23 deletions crates/nargo/src/ops/foreign_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use acvm::{
pwg::ForeignCallWaitInfo,
};
use iter_extended::vecmap;
use noirc_abi::{decode_string_value, input_parser::InputValueDisplay, AbiType};
use noirc_printable_type::{
decode_string_value, decode_value, PrintableType, PrintableValueDisplay,
};
use regex::{Captures, Regex};

use crate::errors::ForeignCallError;
Expand Down Expand Up @@ -93,53 +95,56 @@ impl ForeignCall {
}

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

let input_value_display =
InputValueDisplay::try_from_fields(&mut input_values_as_fields, abi_type)?;
let value = decode_value(&mut input_values_as_fields, &printable_type);
let input_value_display = PrintableValueDisplay::new(&value, &printable_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) =
let (message_as_values, input_and_printable_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 (num_values, input_and_printable_values) = input_and_printable_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);
let mut printable_types = Vec::new();
for printable_value in
input_and_printable_values.iter().skip(input_and_printable_values.len() - num_values)
{
let printable_type = fetch_printable_type(printable_value)?;
printable_types.push(printable_type);
}

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

let mut input_values_as_fields = input_and_abi_values[i..(i + type_size)]
let mut input_values_as_fields = input_and_printable_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())?;
let value = decode_value(&mut input_values_as_fields, printable_type);
let input_value_display = PrintableValueDisplay::new(&value, printable_type);

output_strings.push(input_value_display.to_string());
}
Expand All @@ -157,11 +162,13 @@ fn convert_fmt_string_inputs(
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)
fn fetch_printable_type(
printable_type_as_values: &[Value],
) -> Result<PrintableType, ForeignCallError> {
let printable_type_as_fields = vecmap(printable_type_as_values, |value| value.to_field());
let printable_type_as_string = decode_string_value(&printable_type_as_fields);
let printable_type: PrintableType = serde_json::from_str(&printable_type_as_string)
.map_err(|err| ForeignCallError::InputParserError(err.into()))?;

Ok(abi_type)
Ok(printable_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
75 changes: 74 additions & 1 deletion crates/noirc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use acvm::{
use errors::AbiError;
use input_parser::InputValue;
use iter_extended::{try_btree_map, try_vecmap, vecmap};
use noirc_frontend::{Signedness, Type, TypeBinding, TypeVariableKind, Visibility};
use serde::{Deserialize, Serialize};
// This is the ABI used to bridge the different TOML formats for the initial
// witness, the partial witness generator and the interpreter.
Expand Down Expand Up @@ -74,6 +75,24 @@ pub enum AbiVisibility {
Private,
}

impl From<Visibility> for AbiVisibility {
fn from(value: Visibility) -> Self {
match value {
Visibility::Public => AbiVisibility::Public,
Visibility::Private => AbiVisibility::Private,
}
}
}

impl From<&Visibility> for AbiVisibility {
fn from(value: &Visibility) -> Self {
match value {
Visibility::Public => AbiVisibility::Public,
Visibility::Private => AbiVisibility::Private,
}
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
/// Represents whether the return value should compromise of unique witness indices such that no
Expand All @@ -97,6 +116,60 @@ pub enum Sign {
}

impl AbiType {
// TODO: Add `Context` argument for resolving fully qualified struct paths
phated marked this conversation as resolved.
Show resolved Hide resolved
pub fn from_type(typ: &Type) -> Self {
// Note; use strict_eq instead of partial_eq when comparing field types
// in this method, you most likely want to distinguish between public and private
phated marked this conversation as resolved.
Show resolved Hide resolved
match typ {
Type::FieldElement => Self::Field,
Type::Array(size, typ) => {
let length = size
.evaluate_to_u64()
.expect("Cannot have variable sized arrays as a parameter to main");
let typ = typ.as_ref();
Self::Array { length, typ: Box::new(Self::from_type(typ)) }
}
Type::Integer(sign, bit_width) => {
let sign = match sign {
Signedness::Unsigned => Sign::Unsigned,
Signedness::Signed => Sign::Signed,
};

Self::Integer { sign, width: *bit_width }
}
Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => {
match &*binding.borrow() {
TypeBinding::Bound(typ) => Self::from_type(typ),
TypeBinding::Unbound(_) => Self::from_type(&Type::default_int_type()),
}
}
Type::Bool => Self::Boolean,
Type::String(size) => {
let size = size
.evaluate_to_u64()
.expect("Cannot have variable sized strings as a parameter to main");
Self::String { length: size }
}
Type::FmtString(_, _) => unreachable!("format strings cannot be used in the abi"),
Type::Error => unreachable!(),
Type::Unit => unreachable!(),
Type::Constant(_) => unreachable!(),
Type::Struct(def, ref args) => {
let struct_type = def.borrow();
let fields = struct_type.get_fields(args);
let fields = vecmap(fields, |(name, typ)| (name, Self::from_type(&typ)));
Self::Struct { fields, name: struct_type.name.to_string() }
}
Type::Tuple(_) => todo!("as_abi_type not yet implemented for tuple types"),
phated marked this conversation as resolved.
Show resolved Hide resolved
Type::TypeVariable(_, _) => unreachable!(),
Type::NamedGeneric(..) => unreachable!(),
Type::Forall(..) => unreachable!(),
Type::Function(_, _, _) => unreachable!(),
Type::MutableReference(_) => unreachable!("&mut cannot be used in the abi"),
Type::NotConstant => unreachable!(),
}
}

/// Returns the number of field elements required to represent the type once encoded.
pub fn field_count(&self) -> u32 {
match self {
Expand Down Expand Up @@ -390,7 +463,7 @@ fn decode_value(
Ok(value)
}

pub fn decode_string_value(field_elements: &[FieldElement]) -> String {
fn decode_string_value(field_elements: &[FieldElement]) -> 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
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@

let (parameters, return_type) = func_meta.into_function_signature();
let parameters = into_abi_params(parameters, &context.def_interner);
let return_type = return_type.map(|typ| typ.as_abi_type());
let return_type = return_type.map(|typ| AbiType::from_type(&typ));
Some((parameters, return_type))
}

Expand Down Expand Up @@ -165,7 +165,7 @@
let compiled_program = compile_no_check(context, options, main)?;

if options.print_acir {
println!("Compiled ACIR for main (unoptimized):");

Check warning on line 168 in crates/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unoptimized)
println!("{}", compiled_program.circuit);
}

Expand Down Expand Up @@ -199,7 +199,7 @@
for compiled_contract in &compiled_contracts {
for contract_function in &compiled_contract.functions {
println!(
"Compiled ACIR for {}::{} (unoptimized):",

Check warning on line 202 in crates/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unoptimized)
compiled_contract.name, contract_function.name
);
println!("{}", contract_function.bytecode);
Expand Down
6 changes: 3 additions & 3 deletions crates/noirc_evaluator/src/ssa/abi_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::BTreeMap;

use acvm::acir::native_types::Witness;
use iter_extended::{btree_map, vecmap};
use noirc_abi::{Abi, AbiParameter};
use noirc_abi::{Abi, AbiParameter, AbiType};
use noirc_frontend::{
hir_def::{
function::{FunctionSignature, Param},
Expand All @@ -27,7 +27,7 @@ pub fn into_abi_params(params: Vec<Param>, interner: &NodeInterner) -> Vec<AbiPa
let param_name = get_param_name(&pattern, interner)
.expect("Abi for tuple and struct parameters is unimplemented")
.to_owned();
let as_abi = typ.as_abi_type();
let as_abi = AbiType::from_type(&typ);
AbiParameter { name: param_name, typ: as_abi, visibility: vis.into() }
})
}
Expand All @@ -42,7 +42,7 @@ pub(crate) fn gen_abi(
) -> Abi {
let (parameters, return_type) = func_sig;
let parameters = into_abi_params(parameters, interner);
let return_type = return_type.map(|typ| typ.as_abi_type());
let return_type = return_type.map(|typ| AbiType::from_type(&typ));
let param_witnesses = param_witnesses_from_abi_param(&parameters, input_witnesses);
Abi { parameters, return_type, param_witnesses, return_witnesses }
}
Expand Down
Loading