Add serialisation format of ABIs into JSON (#554)

* feat: add method to serialize a circuit's ABI

* fix: restore generation of Prover.toml and Verifier.toml files

* refactor: improve variable names in build_cmd.rs

* refactor: change enum tag from "name" to "kind"

* refactor: use btree_map in nargo

* style: clippy

* refactor: use vecmap in noirc_abi

* fix: correct new import

* refactor: restrict array lengths to `u64::MAX`

* fix: add missing dependency to `nargo` crate

* refactor: put custom (de-)serializers in module

* refactor: replace accidentally removed docs

* test: move serialization test to module

* refactor: remove unused dep from noirc-abi

Co-authored-by: kevaundray <[email protected]>

crates/nargo/Cargo.toml

@@ -12,6 +12,7 @@ dirs = "4"
 [dependencies]
 dirs = "3.0.1"
 url = "2.2.0"
+iter-extended = { path = "../iter-extended" }
 noirc_driver = { path = "../noirc_driver", features = ["std"] }
 noirc_frontend = { path = "../noirc_frontend" }
 noirc_abi = { path = "../noirc_abi" }

crates/nargo/src/cli/build_cmd.rs

@@ -1,7 +1,12 @@
 use crate::{errors::CliError, resolver::Resolver};
 use acvm::ProofSystemCompiler;
 use clap::ArgMatches;
-use std::path::{Path, PathBuf};
+use iter_extended::btree_map;
+use noirc_abi::{Abi, AbiParameter, AbiType};
+use std::{
+    collections::BTreeMap,
+    path::{Path, PathBuf},
+};
 
 use super::{add_std_lib, write_to_file, PROVER_INPUT_FILE, VERIFIER_INPUT_FILE};
 
@@ -21,7 +26,7 @@ pub fn build_from_path<P: AsRef<Path>>(p: P, allow_warnings: bool) -> Result<(),
     add_std_lib(&mut driver);
     driver.build(allow_warnings);
     // XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files
-    if let Some(x) = driver.compute_abi() {
+    if let Some(abi) = driver.compute_abi() {
         // XXX: The root config should return an enum to determine if we are looking for .json or .toml
         // For now it is hard-coded to be toml.
         //
@@ -33,12 +38,12 @@ pub fn build_from_path<P: AsRef<Path>>(p: P, allow_warnings: bool) -> Result<(),
         // If they are not available, then create them and
         // populate them based on the ABI
         if !path_to_prover_input.exists() {
-            let toml = toml::to_string(&x).unwrap();
+            let toml = toml::to_string(&build_empty_map(abi.clone())).unwrap();
             write_to_file(toml.as_bytes(), &path_to_prover_input);
         }
         if !path_to_verifier_input.exists() {
-            let abi = x.public_abi();
-            let toml = toml::to_string(&abi).unwrap();
+            let public_abi = abi.public_abi();
+            let toml = toml::to_string(&build_empty_map(public_abi)).unwrap();
             write_to_file(toml.as_bytes(), &path_to_verifier_input);
         }
     } else {
@@ -47,6 +52,13 @@ pub fn build_from_path<P: AsRef<Path>>(p: P, allow_warnings: bool) -> Result<(),
     Ok(())
 }
 
+fn build_empty_map(abi: Abi) -> BTreeMap<String, &'static str> {
+    btree_map(abi.parameters, |AbiParameter { name, typ, .. }| {
+        let default_value = if matches!(typ, AbiType::Array { .. }) { "[]" } else { "" };
+        (name, default_value)
+    })
+}
+
 #[cfg(test)]
 mod tests {
     const TEST_DATA_DIR: &str = "tests/build_tests_data";

crates/noirc_abi/Cargo.toml

@@ -12,3 +12,6 @@ toml = "0.5.8"
 serde = { version = "1.0.136", features = ["derive"] }
 serde_derive = "1.0.136"
 blake2 = "0.9.1"
+
+[dev-dependencies]
+serde_json = "1.0"

crates/noirc_abi/src/lib.rs

@@ -3,7 +3,7 @@ use std::{collections::BTreeMap, convert::TryInto};
 use acvm::FieldElement;
 use errors::AbiError;
 use input_parser::InputValue;
-use serde::{ser::SerializeMap, Deserialize, Serialize, Serializer};
+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.
@@ -12,10 +12,12 @@ use serde::{ser::SerializeMap, Deserialize, Serialize, Serializer};
 
 pub mod errors;
 pub mod input_parser;
+mod serialization;
 
 pub const MAIN_RETURN_NAME: &str = "return";
 
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(tag = "kind", rename_all = "lowercase")]
 /// Types that are allowed in the (main function in binary)
 ///
 /// we use this separation so that we can have types like Strings
@@ -28,12 +30,26 @@ pub const MAIN_RETURN_NAME: &str = "return";
 /// support.
 pub enum AbiType {
     Field,
-    Array { length: u128, typ: Box<AbiType> },
-    Integer { sign: Sign, width: u32 },
-    Struct { fields: BTreeMap<String, AbiType> },
+    Array {
+        length: u64,
+        #[serde(rename = "type")]
+        typ: Box<AbiType>,
+    },
+    Integer {
+        sign: Sign,
+        width: u32,
+    },
+    Struct {
+        #[serde(
+            serialize_with = "serialization::serialize_struct_fields",
+            deserialize_with = "serialization::deserialize_struct_fields"
+        )]
+        fields: BTreeMap<String, AbiType>,
+    },
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "lowercase")]
 /// Represents whether the parameter is public or known only to the prover.
 pub enum AbiVisibility {
     Public,
@@ -52,6 +68,7 @@ impl std::fmt::Display for AbiVisibility {
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "lowercase")]
 pub enum Sign {
     Unsigned,
     Signed,
@@ -78,10 +95,11 @@ impl AbiType {
     }
 }
 
-#[derive(Clone, Debug, Deserialize)]
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 /// An argument or return value of the circuit's `main` function.
 pub struct AbiParameter {
     pub name: String,
+    #[serde(rename = "type")]
     pub typ: AbiType,
     pub visibility: AbiVisibility,
 }
@@ -92,7 +110,7 @@ impl AbiParameter {
     }
 }
 
-#[derive(Clone, Debug, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct Abi {
     pub parameters: Vec<AbiParameter>,
 }
@@ -253,22 +271,3 @@ impl Abi {
         Ok((index, value))
     }
 }
-
-impl Serialize for Abi {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: Serializer,
-    {
-        let vec: Vec<u8> = Vec::new();
-        let mut map = serializer.serialize_map(Some(self.parameters.len()))?;
-        for param in &self.parameters {
-            match param.typ {
-                AbiType::Field => map.serialize_entry(&param.name, "")?,
-                AbiType::Array { .. } => map.serialize_entry(&param.name, &vec)?,
-                AbiType::Integer { .. } => map.serialize_entry(&param.name, "")?,
-                AbiType::Struct { .. } => map.serialize_entry(&param.name, "")?,
-            };
-        }
-        map.end()
-    }
-}

crates/noirc_abi/src/serialization.rs

@@ -0,0 +1,139 @@
+use serde::{Deserialize, Deserializer, Serialize, Serializer};
+use std::collections::BTreeMap;
+
+use crate::AbiType;
+
+// This module exposes a custom serializer and deserializer for `BTreeMap<String, AbiType>`
+// (representing the fields of a struct) to serialize it as a `Vec<StructField>`.
+//
+// This is required as the struct is flattened into an array of field elements so the ordering of the struct's fields
+// must be maintained. However, several serialization formats (notably JSON) do not provide strong guarantees about
+// the ordering of elements in a map, this creates potential for improper ABI encoding of structs if the fields are
+// deserialized into a different order. To prevent this, we store the fields in an array to create an unambiguous ordering.
+
+#[derive(Serialize, Deserialize)]
+struct StructField {
+    name: String,
+    #[serde(rename = "type")]
+    typ: AbiType,
+}
+
+pub fn serialize_struct_fields<S>(
+    fields: &BTreeMap<String, AbiType>,
+    s: S,
+) -> Result<S::Ok, S::Error>
+where
+    S: Serializer,
+{
+    let fields_vector: Vec<StructField> = fields
+        .iter()
+        .map(|(name, typ)| StructField { name: name.to_owned(), typ: typ.to_owned() })
+        .collect();
+    fields_vector.serialize(s)
+}
+
+pub fn deserialize_struct_fields<'de, D>(
+    deserializer: D,
+) -> Result<BTreeMap<String, AbiType>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let fields_vector = Vec::<StructField>::deserialize(deserializer)?;
+    let fields = fields_vector.into_iter().map(|StructField { name, typ }| (name, typ)).collect();
+    Ok(fields)
+}
+
+#[cfg(test)]
+mod tests {
+    use std::collections::BTreeMap;
+
+    use crate::{AbiParameter, AbiType, AbiVisibility, Sign};
+
+    #[test]
+    fn abi_parameter_serialization() {
+        let serialized_field = "{
+            \"name\": \"thing1\",
+            \"visibility\": \"public\",
+            \"type\": {
+                \"kind\": \"field\"
+            }
+        }";
+
+        let expected_field = AbiParameter {
+            name: "thing1".to_string(),
+            typ: AbiType::Field,
+            visibility: AbiVisibility::Public,
+        };
+        let deserialized_field: AbiParameter = serde_json::from_str(serialized_field).unwrap();
+        assert_eq!(deserialized_field, expected_field);
+
+        let serialized_array = "{
+            \"name\": \"thing2\",
+            \"visibility\": \"private\",
+            \"type\": {
+                \"kind\": \"array\",
+                \"length\": 2,
+                \"type\": {
+                    \"kind\": \"integer\",
+                    \"width\": 3,
+                    \"sign\": \"unsigned\"
+                }
+            }
+        }";
+
+        let expected_array = AbiParameter {
+            name: "thing2".to_string(),
+            typ: AbiType::Array {
+                length: 2,
+                typ: Box::new(AbiType::Integer { sign: Sign::Unsigned, width: 3 }),
+            },
+            visibility: AbiVisibility::Private,
+        };
+        let deserialized_array: AbiParameter = serde_json::from_str(serialized_array).unwrap();
+        assert_eq!(deserialized_array, expected_array);
+
+        let serialized_struct = "{   
+            \"name\":\"thing3\",
+            \"type\": {
+                \"kind\":\"struct\",
+                \"fields\": [
+                    {
+                        \"name\": \"field1\",
+                        \"type\": {
+                            \"kind\": \"integer\",
+                            \"sign\": \"unsigned\",
+                            \"width\": 3
+                        }
+                    },
+                    {
+                        \"name\":\"field2\",
+                        \"type\": {
+                            \"kind\":\"array\",
+                            \"length\": 2,
+                            \"type\": {
+                                \"kind\":\"field\"
+                            }
+                        }
+                    }
+                ]
+            },
+            \"visibility\":\"private\"
+        }";
+
+        let expected_struct = AbiParameter {
+            name: "thing3".to_string(),
+            typ: AbiType::Struct {
+                fields: BTreeMap::from([
+                    ("field1".to_string(), AbiType::Integer { sign: Sign::Unsigned, width: 3 }),
+                    (
+                        "field2".to_string(),
+                        AbiType::Array { length: 2, typ: Box::new(AbiType::Field) },
+                    ),
+                ]),
+            },
+            visibility: AbiVisibility::Private,
+        };
+        let deserialized_struct: AbiParameter = serde_json::from_str(serialized_struct).unwrap();
+        assert_eq!(deserialized_struct, expected_struct);
+    }
+}

crates/noirc_evaluator/src/lib.rs

@@ -200,7 +200,7 @@ impl Evaluator {
     fn generate_array_witnesses(
         &mut self,
         visibility: &AbiVisibility,
-        length: &u128,
+        length: &u64,
         typ: &AbiType,
     ) -> Result<Vec<Witness>, RuntimeErrorKind> {
         let mut witnesses = Vec::new();

crates/noirc_evaluator/src/ssa/code_gen.rs

@@ -165,7 +165,7 @@ impl IRGenerator {
         name: &str,
         ident_def: Option<DefinitionId>,
         el_type: &noirc_abi::AbiType,
-        len: u128,
+        len: u64,
         witness: Vec<acvm::acir::native_types::Witness>,
     ) -> NodeId {
         let element_type = self.get_object_type_from_abi(el_type);

crates/noirc_frontend/src/hir_def/types.rs

@@ -947,7 +947,7 @@ impl Type {
                 let size = size
                     .array_length()
                     .expect("Cannot have variable sized arrays as a parameter to main");
-                AbiType::Array { length: size as u128, typ: Box::new(typ.as_abi_type()) }
+                AbiType::Array { length: size, typ: Box::new(typ.as_abi_type()) }
             }
             Type::Integer(_, sign, bit_width) => {
                 let sign = match sign {