chore(cli): Use noir_artifact_cli::fs to read artifacts (#7391)

Co-authored-by: Tom French <[email protected]>

acvm-repo/acir/src/circuit/mod.rs

@@ -252,7 +252,7 @@ impl<F: Serialize> Program<F> {
         program_bytes
     }
 
-    // Serialize and base64 encode program
+    /// Serialize and base64 encode program
     pub fn serialize_program_base64<S>(program: &Self, s: S) -> Result<S::Ok, S::Error>
     where
         S: Serializer,
@@ -277,7 +277,7 @@ impl<F: for<'a> Deserialize<'a>> Program<F> {
         Program::read(serialized_circuit)
     }
 
-    // Deserialize and base64 decode program
+    /// Deserialize and base64 decode program
     pub fn deserialize_program_base64<'de, D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: Deserializer<'de>,

compiler/noirc_driver/src/contract.rs

@@ -41,6 +41,8 @@ pub struct CompiledContract {
 pub struct ContractFunction {
     pub name: String,
 
+    pub hash: u64,
+
     pub is_unconstrained: bool,
 
     pub custom_attributes: Vec<String>,

compiler/noirc_driver/src/lib.rs

@@ -549,6 +549,7 @@ fn compile_contract_inner(
 
         functions.push(ContractFunction {
             name,
+            hash: function.hash,
             custom_attributes,
             abi: function.abi,
             bytecode: function.program,

compiler/wasm/src/compile.rs

@@ -13,7 +13,7 @@ use noirc_frontend::{
     hir::Context,
 };
 use serde::Deserialize;
-use std::{collections::HashMap, path::Path};
+use std::{collections::BTreeMap, path::Path};
 use wasm_bindgen::prelude::*;
 
 use crate::errors::{CompileError, JsCompileError};
@@ -128,16 +128,21 @@ impl JsCompileContractResult {
 #[derive(Deserialize, Default)]
 pub(crate) struct DependencyGraph {
     pub(crate) root_dependencies: Vec<CrateName>,
-    pub(crate) library_dependencies: HashMap<CrateName, Vec<CrateName>>,
+    pub(crate) library_dependencies: BTreeMap<CrateName, Vec<CrateName>>,
 }
+/// This is map contains the paths of all of the files in the entry-point crate and
+/// the transitive dependencies of the entry-point crate.
+///
+/// This is for all intents and purposes the file system that the compiler will use to resolve/compile
+/// files in the crate being compiled and its dependencies.
+///
+/// Using a `BTreeMap` to add files to the [FileManager] in a deterministic order,
+/// which affects the `FileId` in the `Location`s in the AST on which the `hash` is based.
+/// Note that we cannot expect to match the IDs assigned by the `FileManager` used by `nargo`,
+/// because there the order is determined by the dependency graph as well as the file name.
 #[wasm_bindgen]
-// This is a map containing the paths of all of the files in the entry-point crate and
-// the transitive dependencies of the entry-point crate.
-//
-// This is for all intents and purposes the file system that the compiler will use to resolve/compile
-// files in the crate being compiled and its dependencies.
 #[derive(Deserialize, Default)]
-pub struct PathToFileSourceMap(pub(crate) HashMap<std::path::PathBuf, String>);
+pub struct PathToFileSourceMap(pub(crate) BTreeMap<std::path::PathBuf, String>);
 
 #[wasm_bindgen]
 impl PathToFileSourceMap {
@@ -207,7 +212,7 @@ pub fn compile_contract(
 
     let compiled_contract =
         noirc_driver::compile_contract(&mut context, crate_id, &compile_options)
-            .map_err(|errs| {
+            .map_err(|errs: Vec<noirc_errors::FileDiagnostic>| {
                 CompileError::with_file_diagnostics(
                     "Failed to compile contract",
                     errs,
@@ -231,7 +236,7 @@ fn prepare_context(
         <JsValue as JsValueSerdeExt>::into_serde(&JsValue::from(dependency_graph))
             .map_err(|err| err.to_string())?
     } else {
-        DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() }
+        DependencyGraph { root_dependencies: vec![], library_dependencies: BTreeMap::new() }
     };
 
     let fm = file_manager_with_source_map(file_source_map);
@@ -272,7 +277,7 @@ pub(crate) fn file_manager_with_source_map(source_map: PathToFileSourceMap) -> F
 // upon some library `lib1`. Then the packages that `lib1` depend upon will be placed in the
 // `library_dependencies` list and the `lib1` will be placed in the `root_dependencies` list.
 fn process_dependency_graph(context: &mut Context, dependency_graph: DependencyGraph) {
-    let mut crate_names: HashMap<&CrateName, CrateId> = HashMap::new();
+    let mut crate_names: BTreeMap<&CrateName, CrateId> = BTreeMap::new();
 
     for lib in &dependency_graph.root_dependencies {
         let crate_id = add_noir_lib(context, lib);
@@ -312,7 +317,7 @@ mod test {
     use crate::compile::PathToFileSourceMap;
 
     use super::{file_manager_with_source_map, process_dependency_graph, DependencyGraph};
-    use std::{collections::HashMap, path::Path};
+    use std::{collections::BTreeMap, path::Path};
 
     fn setup_test_context(source_map: PathToFileSourceMap) -> Context<'static, 'static> {
         let mut fm = file_manager_with_source_map(source_map);
@@ -333,7 +338,7 @@ mod test {
     #[test]
     fn test_works_with_empty_dependency_graph() {
         let dependency_graph =
-            DependencyGraph { root_dependencies: vec![], library_dependencies: HashMap::new() };
+            DependencyGraph { root_dependencies: vec![], library_dependencies: BTreeMap::new() };
 
         let source_map = PathToFileSourceMap::default();
         let mut context = setup_test_context(source_map);
@@ -348,7 +353,7 @@ mod test {
     fn test_works_with_root_dependencies() {
         let dependency_graph = DependencyGraph {
             root_dependencies: vec![crate_name("lib1")],
-            library_dependencies: HashMap::new(),
+            library_dependencies: BTreeMap::new(),
         };
 
         let source_map = PathToFileSourceMap(
@@ -368,7 +373,7 @@ mod test {
     fn test_works_with_duplicate_root_dependencies() {
         let dependency_graph = DependencyGraph {
             root_dependencies: vec![crate_name("lib1"), crate_name("lib1")],
-            library_dependencies: HashMap::new(),
+            library_dependencies: BTreeMap::new(),
         };
 
         let source_map = PathToFileSourceMap(
@@ -387,7 +392,7 @@ mod test {
     fn test_works_with_transitive_dependencies() {
         let dependency_graph = DependencyGraph {
             root_dependencies: vec![crate_name("lib1")],
-            library_dependencies: HashMap::from([
+            library_dependencies: BTreeMap::from([
                 (crate_name("lib1"), vec![crate_name("lib2")]),
                 (crate_name("lib2"), vec![crate_name("lib3")]),
             ]),
@@ -413,7 +418,7 @@ mod test {
     fn test_works_with_missing_dependencies() {
         let dependency_graph = DependencyGraph {
             root_dependencies: vec![crate_name("lib1")],
-            library_dependencies: HashMap::from([(crate_name("lib2"), vec![crate_name("lib3")])]),
+            library_dependencies: BTreeMap::from([(crate_name("lib2"), vec![crate_name("lib3")])]),
         };
 
         let source_map = PathToFileSourceMap(

compiler/wasm/src/types/noir_artifact.ts

@@ -58,6 +58,8 @@ export interface ABIVariable {
 export interface NoirFunctionEntry {
   /** The name of the function. */
   name: string;
+  /** The hash of the circuit. */
+  hash?: string;
   /** Whether the function is unconstrained. */
   is_unconstrained: boolean;
   /** The custom attributes applied to the function. */
@@ -96,7 +98,7 @@ export interface ProgramArtifact {
   /** Version of noir used for the build. */
   noir_version: string;
   /** The hash of the circuit. */
-  hash?: number;
+  hash?: string;
   /** * The ABI of the function. */
   abi: Abi;
   /** The bytecode of the circuit in base64. */

compiler/wasm/test/compiler/shared/compile.test.ts

@@ -27,18 +27,19 @@ export function shouldCompileProgramIdentically(
     // Prepare noir-wasm artifact
     const noirWasmProgram = noirWasmArtifact.program;
     expect(noirWasmProgram).not.to.be.undefined;
-    const [_noirWasmDebugInfos, norWasmFileMap] = deleteProgramDebugMetadata(noirWasmProgram);
+    const [_noirWasmDebugInfos, noirWasmFileMap] = deleteProgramDebugMetadata(noirWasmProgram);
     normalizeVersion(noirWasmProgram);
 
-    // We first compare both contracts without considering debug info
+    // We first compare both contracts without considering debug info.
+    // We can't expect hashes to match `nargo` because of the different order in which dependencies are visited.
     delete (noirWasmProgram as Partial<ProgramArtifact>).hash;
     delete (nargoArtifact as Partial<ProgramArtifact>).hash;
     expect(nargoArtifact).to.deep.eq(noirWasmProgram);
 
     // Compare the file maps, ignoring keys, since those depend in the order in which files are visited,
     // which may change depending on the file manager implementation. Also ignores paths, since the base
     // path is reported differently between nargo and noir-wasm.
-    expect(getSources(nargoFileMap)).to.have.members(getSources(norWasmFileMap));
+    expect(getSources(nargoFileMap)).to.have.members(getSources(noirWasmFileMap));
 
     // Compare the debug symbol information, ignoring the actual ids used for file identifiers.
     // Debug symbol info looks like the following, what we need is to ignore the 'file' identifiers
@@ -63,16 +64,23 @@ export function shouldCompileContractIdentically(
     // Prepare noir-wasm artifact
     const noirWasmContract = noirWasmArtifact.contract;
     expect(noirWasmContract).not.to.be.undefined;
-    const [noirWasmDebugInfos, norWasmFileMap] = deleteContractDebugMetadata(noirWasmContract);
+    const [noirWasmDebugInfos, noirWasmFileMap] = deleteContractDebugMetadata(noirWasmContract);
     normalizeVersion(noirWasmContract);
 
     // We first compare both contracts without considering debug info
+    // We can't expect hashes to match `nargo` because of the different order in which dependencies are visited.
+    nargoArtifact.functions.forEach(function (f) {
+      delete (f as Partial<NoirFunctionEntry>).hash;
+    });
+    noirWasmContract.functions.forEach(function (f) {
+      delete (f as Partial<NoirFunctionEntry>).hash;
+    });
     expect(nargoArtifact).to.deep.eq(noirWasmContract);
 
     // Compare the file maps, ignoring keys, since those depend in the order in which files are visited,
     // which may change depending on the file manager implementation. Also ignores paths, since the base
     // path is reported differently between nargo and noir-wasm.
-    expect(getSources(nargoFileMap)).to.have.members(getSources(norWasmFileMap));
+    expect(getSources(nargoFileMap)).to.have.members(getSources(noirWasmFileMap));
 
     // Compare the debug symbol information, ignoring the actual ids used for file identifiers.
     // Debug symbol info looks like the following, what we need is to ignore the 'file' identifiers

tooling/artifact_cli/src/fs/artifact.rs

@@ -1,16 +1,18 @@
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use crate::{
     errors::{CliError, FilesystemError},
     Artifact,
 };
 use noirc_artifacts::contract::ContractArtifact;
 use noirc_artifacts::program::ProgramArtifact;
+use noirc_driver::CrateName;
+use serde::de::Error;
 
 impl Artifact {
     /// Try to parse an artifact as a binary program or a contract
     pub fn read_from_file(path: &Path) -> Result<Self, CliError> {
-        let json = std::fs::read(path).map_err(FilesystemError::from)?;
+        let json = std::fs::read(path.with_extension("json")).map_err(FilesystemError::from)?;
 
         let as_program = || serde_json::from_slice::<ProgramArtifact>(&json).map(Artifact::Program);
         let as_contract =
@@ -35,3 +37,55 @@ pub fn read_bytecode_from_file(
         .map_err(|e| FilesystemError::InvalidBytecodeFile(file_path, e.to_string()))?;
     Ok(bytecode)
 }
+
+/// Read a `ProgramArtifact`. Returns error if it turns out to be a `ContractArtifact`.
+pub fn read_program_from_file(path: &Path) -> Result<ProgramArtifact, CliError> {
+    match Artifact::read_from_file(path)? {
+        Artifact::Program(program) => Ok(program),
+        Artifact::Contract(contract) => {
+            let msg = format!(
+                "expected a program artifact but found a contract in {}: {}",
+                path.display(),
+                contract.name
+            );
+            Err(CliError::ArtifactDeserializationError(serde_json::Error::custom(msg)))
+        }
+    }
+}
+
+pub fn save_program_to_file(
+    program_artifact: &ProgramArtifact,
+    crate_name: &CrateName,
+    output_dir: &Path,
+) -> Result<PathBuf, CliError> {
+    let circuit_name: String = crate_name.into();
+    save_build_artifact_to_file(program_artifact, &circuit_name, output_dir)
+}
+
+pub fn save_contract_to_file(
+    compiled_contract: &ContractArtifact,
+    circuit_name: &str,
+    output_dir: &Path,
+) -> Result<PathBuf, CliError> {
+    save_build_artifact_to_file(compiled_contract, circuit_name, output_dir)
+}
+
+fn save_build_artifact_to_file<T: ?Sized + serde::Serialize>(
+    build_artifact: &T,
+    artifact_name: &str,
+    output_dir: &Path,
+) -> Result<PathBuf, CliError> {
+    let artifact_path = output_dir.join(artifact_name).with_extension("json");
+    let bytes = serde_json::to_vec(build_artifact)?;
+    write_to_file(&bytes, &artifact_path)?;
+    Ok(artifact_path)
+}
+
+/// Create the parent directory if needed and write the bytes to a file.
+pub fn write_to_file(bytes: &[u8], path: &Path) -> Result<(), FilesystemError> {
+    if let Some(dir) = path.parent() {
+        std::fs::create_dir_all(dir)?;
+    }
+    std::fs::write(path, bytes)?;
+    Ok(())
+}

tooling/artifact_cli/src/fs/witness.rs

@@ -3,14 +3,14 @@ use std::path::{Path, PathBuf};
 use acir::{native_types::WitnessStackError, FieldElement};
 use acvm::acir::native_types::WitnessStack;
 
-use crate::errors::FilesystemError;
+use crate::errors::{CliError, FilesystemError};
 
 /// Write `witness.gz` to the output directory.
 pub fn save_witness_to_dir(
     witnesses: WitnessStack<FieldElement>,
     witness_name: &str,
     witness_dir: &Path,
-) -> Result<PathBuf, FilesystemError> {
+) -> Result<PathBuf, CliError> {
     std::fs::create_dir_all(witness_dir)?;
 
     let witness_path = witness_dir.join(witness_name).with_extension("gz");

tooling/inspector/Cargo.toml

@@ -26,3 +26,4 @@ const_format.workspace = true
 acir.workspace = true
 noirc_artifacts.workspace = true
 noirc_artifacts_info.workspace = true
+noir_artifact_cli.workspace = true