feat: Changes serialization for contract functions (#1056)

* use function as a Vector instead of BTreeMap

* refactor code to use contract function vec instead of map

* use new ContractFunction structure

* add verification key

* refactor to use contract function

* separate acir hashing from saving to directory

* modify test

* separate preprocessing from saving to disk

* refactor code to use Preprocessed data

* use From impl

* Update crates/nargo_cli/src/cli/preprocess_cmd.rs

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

* Update crates/noirc_driver/src/contract.rs

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

* Update crates/noirc_driver/src/lib.rs

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

* preprocess all contract data beforehand

* fix vecmap

* Update crates/nargo_cli/src/cli/compile_cmd.rs

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

crates/nargo_cli/src/cli/compile_cmd.rs

@@ -8,7 +8,7 @@ use crate::resolver::DependencyResolutionError;
 use crate::{constants::TARGET_DIR, errors::CliError, resolver::Resolver};
 
 use super::fs::program::{save_contract_to_file, save_program_to_file};
-use super::preprocess_cmd::preprocess_with_path;
+use super::preprocess_cmd::{save_preprocess_data, PreprocessedData};
 use super::NargoConfig;
 
 /// Compile the program and its secret execution trace into ACIR format
@@ -31,10 +31,10 @@ pub(crate) fn run(args: CompileCommand, config: NargoConfig) -> Result<(), CliEr
     // If contracts is set we're compiling every function in a 'contract' rather than just 'main'.
     if args.contracts {
         let mut driver = setup_driver(&config.program_dir)?;
-        let compiled_contracts = driver
+        let mut compiled_contracts = driver
             .compile_contracts(&args.compile_options)
             .map_err(|_| CliError::CompilationError)?;
-        save_and_preprocess_contract(&compiled_contracts, &args.circuit_name, &circuit_dir)
+        save_and_preprocess_contract(&mut compiled_contracts, &args.circuit_name, &circuit_dir)
     } else {
         let program = compile_circuit(&config.program_dir, &args.compile_options)?;
         save_and_preprocess_program(&program, &args.circuit_name, &circuit_dir)
@@ -53,37 +53,61 @@ fn save_and_preprocess_program(
     circuit_dir: &Path,
 ) -> Result<(), CliError> {
     save_program_to_file(compiled_program, circuit_name, circuit_dir);
-    preprocess_with_path(circuit_name, circuit_dir, &compiled_program.circuit)?;
+
+    let preprocessed_data = PreprocessedData::from(&compiled_program.circuit);
+    save_preprocess_data(&preprocessed_data, circuit_name, circuit_dir)?;
     Ok(())
 }
 
 /// Save a contract to disk along with proving and verification keys.
 /// - The contract ABI is saved as one file, which contains all of the
 /// functions defined in the contract.
 /// - The proving and verification keys are namespaced since the file
-/// could contain multiple contracts with the same name.
+/// could contain multiple contracts with the same name. The verification key is saved inside
+/// of the ABI.
 fn save_and_preprocess_contract(
-    compiled_contracts: &[CompiledContract],
+    compiled_contracts: &mut [CompiledContract],
     circuit_name: &str,
     circuit_dir: &Path,
 ) -> Result<(), CliError> {
     for compiled_contract in compiled_contracts {
+        // Preprocess all contract data
+        // We are patching the verification key in our contract functions
+        // so when we save it to disk, the ABI will have the verification key.
+        let mut contract_preprocess_data = Vec::new();
+        for contract_function in &mut compiled_contract.functions {
+            let preprocessed_data = PreprocessedData::from(&contract_function.bytecode);
+            contract_function.verification_key = Some(preprocessed_data.verification_key.clone());
+            contract_preprocess_data.push(preprocessed_data);
+        }
+
         // Unique identifier for a contract.
         let contract_id = format!("{}-{}", circuit_name, &compiled_contract.name);
 
         // Save contract ABI to file using the contract ID.
+        // This includes the verification keys for each contract function.
         save_contract_to_file(compiled_contract, &contract_id, circuit_dir);
 
-        for (function_name, contract_function) in &compiled_contract.functions {
+        // Save preprocessed data to disk
+        //
+        // TODO: This also includes the verification key, for now we save it in twice
+        // TODO, once in ABI and once to disk as we did before.
+        // TODO: A possible fix is to use optional fields in PreprocessedData
+        // TODO struct. Then make VK None before saving so it is not saved to disk
+        for (contract_function, preprocessed_data) in
+            compiled_contract.functions.iter().zip(contract_preprocess_data)
+        {
             // Create a name which uniquely identifies this contract function
             // over multiple contracts.
-            let uniquely_identifying_program_name = format!("{}-{}", contract_id, function_name);
+            let uniquely_identifying_program_name =
+                format!("{}-{}", contract_id, contract_function.name);
             // Each program in a contract is preprocessed
             // Note: This can potentially be quite a long running process
-            preprocess_with_path(
+
+            save_preprocess_data(
+                &preprocessed_data,
                 &uniquely_identifying_program_name,
                 circuit_dir,
-                &contract_function.function.circuit,
             )?;
         }
     }

crates/nargo_cli/src/cli/fs/keys.rs

@@ -62,7 +62,10 @@ pub(crate) fn fetch_pk_and_vk<P: AsRef<Path>>(
 #[cfg(test)]
 mod tests {
     use super::fetch_pk_and_vk;
-    use crate::cli::fs::{keys::save_key_to_dir, program::save_acir_hash_to_dir};
+    use crate::cli::fs::{
+        keys::save_key_to_dir,
+        program::{hash_acir, save_acir_hash_to_dir},
+    };
     use acvm::acir::circuit::Circuit;
     use tempdir::TempDir;
 
@@ -78,7 +81,7 @@ mod tests {
         save_key_to_dir(&pk, circuit_name, &circuit_build_path, true).unwrap();
         save_key_to_dir(&vk, circuit_name, &circuit_build_path, false).unwrap();
 
-        save_acir_hash_to_dir(&circuit, circuit_name, &circuit_build_path);
+        save_acir_hash_to_dir(hash_acir(&circuit), circuit_name, &circuit_build_path);
         circuit_build_path.push(circuit_name);
 
         let loaded_keys = fetch_pk_and_vk(&circuit, circuit_build_path, true, true).unwrap();

crates/nargo_cli/src/cli/fs/program.rs

@@ -34,12 +34,14 @@ fn save_build_artifact_to_file<P: AsRef<Path>, T: ?Sized + serde::Serialize>(
     circuit_path
 }
 
+pub(crate) fn hash_acir(circuit: &Circuit) -> [u8; 32] {
+    hash_constraint_system(circuit)
+}
 pub(crate) fn save_acir_hash_to_dir<P: AsRef<Path>>(
-    circuit: &Circuit,
+    acir_hash: [u8; 32],
     hash_name: &str,
     hash_dir: P,
 ) -> PathBuf {
-    let acir_hash = hash_constraint_system(circuit);
     let hash_path = hash_dir.as_ref().join(hash_name).with_extension(ACIR_CHECKSUM);
     write_to_file(hex::encode(acir_hash).as_bytes(), &hash_path);
 

crates/nargo_cli/src/cli/preprocess_cmd.rs

@@ -8,7 +8,7 @@ use crate::{constants::TARGET_DIR, errors::CliError};
 
 use super::fs::{
     keys::save_key_to_dir,
-    program::{read_program_from_file, save_acir_hash_to_dir},
+    program::{hash_acir, read_program_from_file, save_acir_hash_to_dir},
 };
 use super::NargoConfig;
 
@@ -23,26 +23,45 @@ pub(crate) fn run(args: PreprocessCommand, config: NargoConfig) -> Result<(), Cl
     let circuit_dir = config.program_dir.join(TARGET_DIR);
 
     let program = read_program_from_file(circuit_dir.join(&args.artifact_name))?;
-
-    preprocess_with_path(&args.artifact_name, circuit_dir, &program.circuit)?;
+    let preprocess_data = PreprocessedData::from(&program.circuit);
+    save_preprocess_data(&preprocess_data, &args.artifact_name, circuit_dir)?;
 
     Ok(())
 }
+/// The result of preprocessing the ACIR bytecode.
+/// The proving, verification key and circuit are backend specific.
+///
+/// The circuit is backend specific because at the end of compilation
+/// an optimization pass is applied which will transform the bytecode into
+/// a format that the backend will accept; removing unsupported gates
+/// is one example of this.
+pub(crate) struct PreprocessedData {
+    pub(crate) proving_key: Vec<u8>,
+    pub(crate) verification_key: Vec<u8>,
+    pub(crate) program_hash: [u8; 32],
+}
+
+impl From<&Circuit> for PreprocessedData {
+    fn from(circuit: &Circuit) -> Self {
+        let backend = crate::backends::ConcreteBackend;
+        let (proving_key, verification_key) = backend.preprocess(circuit);
+        let program_hash = hash_acir(circuit);
+
+        PreprocessedData { proving_key, verification_key, program_hash }
+    }
+}
 
-pub(crate) fn preprocess_with_path<P: AsRef<Path>>(
+pub(crate) fn save_preprocess_data<P: AsRef<Path>>(
+    data: &PreprocessedData,
     key_name: &str,
     preprocess_dir: P,
-    circuit: &Circuit,
 ) -> Result<(PathBuf, PathBuf), CliError> {
-    let backend = crate::backends::ConcreteBackend;
-    let (proving_key, verification_key) = backend.preprocess(circuit);
-
     // Save a checksum of the circuit to compare against during proving and verification.
     // If hash doesn't match then the circuit has been updated and keys are stale.
-    save_acir_hash_to_dir(circuit, key_name, &preprocess_dir);
+    save_acir_hash_to_dir(data.program_hash, key_name, &preprocess_dir);
 
-    let pk_path = save_key_to_dir(&proving_key, key_name, &preprocess_dir, true)?;
-    let vk_path = save_key_to_dir(&verification_key, key_name, preprocess_dir, false)?;
+    let pk_path = save_key_to_dir(&data.proving_key, key_name, &preprocess_dir, true)?;
+    let vk_path = save_key_to_dir(&data.verification_key, key_name, preprocess_dir, false)?;
 
     Ok((pk_path, vk_path))
 }

crates/noirc_driver/src/contract.rs

@@ -1,25 +1,13 @@
-use std::collections::BTreeMap;
-
-use crate::CompiledProgram;
-
-/// Each function in the contract will be compiled
-/// as a separate noir program.
-///
-/// A contract function unlike a regular Noir program
-/// however can have addition properties.
-/// One of these being a function type.
-#[derive(serde::Serialize, serde::Deserialize)]
-pub struct ContractFunction {
-    pub func_type: ContractFunctionType,
-    pub function: CompiledProgram,
-}
+use acvm::acir::circuit::Circuit;
+use noirc_abi::Abi;
+use serde::{Deserialize, Serialize};
 
 /// Describes the types of smart contract functions that are allowed.
 /// Unlike the similar enum in noirc_frontend, 'open' and 'unconstrained'
 /// are mutually exclusive here. In the case a function is both, 'unconstrained'
 /// takes precedence.
 #[derive(serde::Serialize, serde::Deserialize, Debug, Copy, Clone, PartialEq, Eq)]
-#[serde(rename_all = "snake_case")]
+#[serde(rename_all = "camelCase")]
 pub enum ContractFunctionType {
     /// This function will be executed in a private
     /// context.
@@ -33,12 +21,38 @@ pub enum ContractFunctionType {
 }
 
 #[derive(serde::Serialize, serde::Deserialize)]
+#[serde(rename_all = "camelCase")]
 pub struct CompiledContract {
     /// The name of the contract.
     pub name: String,
     /// Each of the contract's functions are compiled into a separate `CompiledProgram`
-    /// stored in this `BTreeMap`.
-    pub functions: BTreeMap<String, ContractFunction>,
+    /// stored in this `Vector`.
+    pub functions: Vec<ContractFunction>,
+}
+
+/// Each function in the contract will be compiled
+/// as a separate noir program.
+///
+/// A contract function unlike a regular Noir program
+/// however can have additional properties.
+/// One of these being a function type.
+#[derive(Debug, Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct ContractFunction {
+    pub name: String,
+
+    pub function_type: ContractFunctionType,
+
+    #[serde(flatten)]
+    pub abi: Abi,
+
+    #[serde(
+        serialize_with = "crate::program::serialize_circuit",
+        deserialize_with = "crate::program::deserialize_circuit"
+    )]
+    pub bytecode: Circuit,
+
+    pub verification_key: Option<Vec<u8>>,
 }
 
 impl ContractFunctionType {

crates/noirc_driver/src/lib.rs

@@ -6,7 +6,7 @@ use acvm::Language;
 use clap::Args;
 use contract::{ContractFunction, ContractFunctionType};
 use fm::FileType;
-use iter_extended::{try_btree_map, try_vecmap};
+use iter_extended::{try_vecmap, vecmap};
 use noirc_abi::FunctionSignature;
 use noirc_errors::{reporter, ReportedError};
 use noirc_evaluator::create_circuit;
@@ -202,7 +202,7 @@ impl Driver {
         contract: Contract,
         options: &CompileOptions,
     ) -> Result<CompiledContract, ReportedError> {
-        let functions = try_btree_map(&contract.functions, |function_id| {
+        let functions = try_vecmap(&contract.functions, |function_id| {
             let function_name = self.function_name(*function_id).to_owned();
             let function = self.compile_no_check(options, *function_id)?;
             let func_meta = self.context.def_interner.function_meta(function_id);
@@ -212,10 +212,21 @@ impl Driver {
 
             let func_type = ContractFunctionType::new(func_type, func_meta.is_unconstrained);
 
-            Ok((function_name, ContractFunction { func_type, function }))
+            Ok((function_name, func_type, function))
         })?;
 
-        Ok(CompiledContract { name: contract.name, functions })
+        let converted_functions =
+            vecmap(functions, |(name, function_type, function)| ContractFunction {
+                name,
+                function_type,
+                abi: function.abi,
+                bytecode: function.circuit,
+                // Since we have not called the proving system yet
+                // we do not have a verification key
+                verification_key: None,
+            });
+
+        Ok(CompiledContract { name: contract.name, functions: converted_functions })
     }
 
     /// Returns the FuncId of the 'main' function.

crates/noirc_driver/src/program.rs

@@ -9,7 +9,7 @@ pub struct CompiledProgram {
     pub abi: noirc_abi::Abi,
 }
 
-fn serialize_circuit<S>(circuit: &Circuit, s: S) -> Result<S::Ok, S::Error>
+pub(crate) fn serialize_circuit<S>(circuit: &Circuit, s: S) -> Result<S::Ok, S::Error>
 where
     S: Serializer,
 {
@@ -19,7 +19,7 @@ where
     circuit_bytes.serialize(s)
 }
 
-fn deserialize_circuit<'de, D>(deserializer: D) -> Result<Circuit, D::Error>
+pub(crate) fn deserialize_circuit<'de, D>(deserializer: D) -> Result<Circuit, D::Error>
 where
     D: Deserializer<'de>,
 {

crates/wasm/src/compile.rs

@@ -100,9 +100,9 @@ pub fn compile(args: JsValue) -> JsValue {
             .into_iter()
             .flat_map(|contract| {
                 let contract_id = format!("{}-{}", options.circuit_name, &contract.name);
-                contract.functions.into_iter().map(move |(function, program)| {
-                    let program_name = format!("{}-{}", contract_id, function);
-                    (program_name, program)
+                contract.functions.into_iter().map(move |contract_function| {
+                    let program_name = format!("{}-{}", contract_id, contract_function.name);
+                    (program_name, contract_function.bytecode)
                 })
             })
             .collect();