refactor: BonsolCli parsing options and error handling (#73)

Ref #35 

Adds better error handling, error messages for some cases, leaving the
cli usage mostly untouched. Adds validation stage for parsing the deploy
args. Adds some nixos stuff that went out of date due to downstream
changes.

CHANGELOG.md

@@ -5,19 +5,21 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
+
+### Changed
+* `bonsol` cli option requirements and error messages updated for added clarity
+
+### Fixed
 * **Breaking**: `execute_v1` interface instruction now uses the new `InputRef` to improve CU usage.
 * Adds a callback struct to use the input_hash and committed_outputs from the callback program ergonomically.
 * Fixes requester/payer mismatch in the node account selection
 * **Breaking**: Forwards input hash to the callback program in all cases.
 * **Breaking**: Changes flatbuffer `Account` struct to have 8 byte alignment due a possible bug in the flatbufers compiler. [https://github.com/google/flatbuffers/pull/8398](Bug Here)
 * **Breaking**: Flatbuffers was upgraded to `24.3.25`
 * `risc0-groth16-prover` binaries (rapidsnark & stark-verify) are available to the nix store, partially unblocking NixOS support.
-* Fixed alignment of `Account` struct in the schemas.
 * `flatbuffers` code is now dynamically generated at build time
 * Fixed alignment of `Account` struct in the schemas.
 
-
-
 ## [0.2.1] - 2024-10-13
 
 ### Changed

cli/Cargo.toml

@@ -22,7 +22,7 @@ hex = "0.4.3"
 byte-unit = "4.0.19"
 bytes = "1.4.0"
 cargo_toml = "0.20.3"
-clap = { version = "4.4.2", features = ["derive"] }
+clap = { version = "4.4.2", features = ["derive", "env"] }
 indicatif = "0.17.8"
 num-traits = "0.2.15"
 object_store = { version = "0.9.1", features = ["aws"] }
@@ -43,6 +43,7 @@ solana-cli-config = { workspace = true }
 solana-rpc-client = { workspace = true }
 solana-sdk = { workspace = true }
 tera = "1.17.1"
+thiserror = "1.0.65"
 tokio = { version = "1.38.0", features = ["full"] }
 
 bonsol-interface.workspace = true

cli/README.md

@@ -43,12 +43,12 @@ The output of the build command is a manifest.json file which is placed in the r
 You can deploy a bonsol program with the following command
 
 ```
-bonsol -k ./keypair.json -u http://localhost:8899  deploy -m {path to manifest.json} -t {type of deployment} -y {auto confirm} ... {upload type specific options}
+bonsol -k ./keypair.json -u http://localhost:8899 deploy -m {path to manifest.json} -y {auto confirm} -t {s3|shadow-drive|url} ... {upload type specific options}
 
 ```
 There will be many options for how to upload the program, the default is s3. Here is an example of how to deploy a program to s3
 ```
-bonsol -k ./keypair.json -u http://localhost:8899  deploy -m program/manifest.json -t s3 --bucket bonsol-public-images --region us-east-1 --access-key {your key} --secret-key {your secret key}
+bonsol -k ./keypair.json -u http://localhost:8899 deploy -m program/manifest.json -t s3 --bucket bonsol-public-images --region us-east-1 --access-key {your key} --secret-key {your secret key}
 ```
 In the above example the manifest.json file is the file that was created by the build command.
 This will try to upload the binary to the s3 bucket and create a deployment account for the program. Programs are indexed by the image id, which is a kind of checksum of the program elf file. This means that if you change the elf file, the image id will change and the program will be deployed again under a new deployment account. Programs are immutable and can only be changed by redeploying the program. When a node downloads a program it will check the image id and if it doesnt match the deployment account it will reject the program. Furthermore when bonsol checks the proof, it will check the image id and if it doesnt match the deployment account and desired image id from execution request it will reject the proof.

cli/src/build.rs

@@ -3,109 +3,202 @@ use std::path::Path;
 use std::process::Command;
 use std::time::Duration;
 
-use crate::common::*;
 use anyhow::Result;
+use cargo_toml::Manifest;
 use indicatif::ProgressBar;
 use risc0_zkvm::compute_image_id;
 use solana_sdk::signer::Signer;
 
+use crate::common::*;
+use crate::error::{BonsolCliError, ZkManifestError};
+
 pub fn build(keypair: &impl Signer, zk_program_path: String) -> Result<()> {
+    validate_build_dependencies()?;
+
     let bar = ProgressBar::new_spinner();
     bar.enable_steady_tick(Duration::from_millis(100));
+
     let image_path = Path::new(&zk_program_path);
-    // ensure cargo risc0 is installed and has the plugin
-    if !cargo_has_plugin("risczero") || !cargo_has_plugin("binstall") || !has_executable("docker") {
-        bar.finish_and_clear();
-        return Err(anyhow::anyhow!(
-            "Please install cargo-risczero and cargo-binstall and docker"
-        ));
-    }
+    let (cargo_package_name, input_order) = parse_cargo_manifest(image_path)?;
+    let build_result =
+        build_zkprogram_manifest(image_path, &keypair, cargo_package_name, input_order);
+    let manifest_path = image_path.join(MANIFEST_JSON);
 
-    let build_result = build_maifest(image_path, &keypair);
-    let manifest_path = image_path.join("manifest.json");
     match build_result {
         Err(e) => {
-            bar.finish_with_message(format!("Error building image: {:?}", e));
+            bar.finish_with_message(format!(
+                "Build failed for program '{}': {:?}",
+                image_path.to_string_lossy(),
+                e
+            ));
             Ok(())
         }
         Ok(manifest) => {
-            serde_json::to_writer_pretty(File::create(&manifest_path).unwrap(), &manifest).unwrap();
+            serde_json::to_writer_pretty(File::create(&manifest_path)?, &manifest)?;
             bar.finish_and_clear();
+            println!("Build complete");
             Ok(())
         }
     }
 }
 
-fn build_maifest(
-    image_path: &Path,
-    keypair: &impl Signer,
-) -> Result<ZkProgramManifest, std::io::Error> {
-    let manifest_path = image_path.join("Cargo.toml");
-    let manifest = cargo_toml::Manifest::from_path(&manifest_path).unwrap();
-    let package = manifest
+fn validate_build_dependencies() -> Result<(), BonsolCliError> {
+    const CARGO_RISCZERO: &str = "risczero";
+    const DOCKER: &str = "docker";
+
+    let mut missing_deps = Vec::with_capacity(2);
+
+    if !cargo_has_plugin(CARGO_RISCZERO) {
+        missing_deps.push(format!("cargo-{}", CARGO_RISCZERO));
+    }
+    if !has_executable(DOCKER) {
+        missing_deps.push(DOCKER.into());
+    }
+
+    if !missing_deps.is_empty() {
+        return Err(BonsolCliError::MissingBuildDependencies { missing_deps });
+    }
+
+    Ok(())
+}
+
+fn parse_cargo_manifest_inputs(
+    manifest: &Manifest,
+    manifest_path_str: String,
+) -> Result<Vec<String>> {
+    const METADATA: &str = "metadata";
+    const ZKPROGRAM: &str = "zkprogram";
+    const INPUT_ORDER: &str = "input_order";
+
+    let meta = manifest
         .package
         .as_ref()
-        .map(|p| &p.name)
-        .ok_or(std::io::Error::new(
-            std::io::ErrorKind::Other,
-            "Invalid Cargo.toml",
+        .and_then(|p| p.metadata.as_ref())
+        .ok_or(ZkManifestError::MissingPackageMetadata(
+            manifest_path_str.clone(),
         ))?;
-    let meta = manifest.package.as_ref().and_then(|p| p.metadata.as_ref());
-    if meta.is_none() {
-        return Err(std::io::Error::new(
-            std::io::ErrorKind::Other,
-            "Invalid Cargo.toml, missing package metadata",
-        ));
+    let meta_table = meta.as_table().ok_or(ZkManifestError::ExpectedTable {
+        manifest_path: manifest_path_str.clone(),
+        name: METADATA.into(),
+    })?;
+    let zkprogram = meta_table
+        .get(ZKPROGRAM)
+        .ok_or(ZkManifestError::MissingProgramMetadata {
+            manifest_path: manifest_path_str.clone(),
+            meta: meta.to_owned(),
+        })?;
+    let zkprogram_table = zkprogram.as_table().ok_or(ZkManifestError::ExpectedTable {
+        manifest_path: manifest_path_str.clone(),
+        name: ZKPROGRAM.into(),
+    })?;
+    let input_order =
+        zkprogram_table
+            .get(INPUT_ORDER)
+            .ok_or(ZkManifestError::MissingInputOrder {
+                manifest_path: manifest_path_str.clone(),
+                zkprogram: zkprogram.to_owned(),
+            })?;
+    let inputs = input_order
+        .as_array()
+        .ok_or(ZkManifestError::ExpectedArray {
+            manifest_path: manifest_path_str.clone(),
+            name: INPUT_ORDER.into(),
+        })?;
+
+    let (input_order, errs): (
+        Vec<Result<String, ZkManifestError>>,
+        Vec<Result<String, ZkManifestError>>,
+    ) = inputs
+        .iter()
+        .map(|i| -> Result<String, ZkManifestError> {
+            i.as_str()
+                .map(|s| s.to_string())
+                .ok_or(ZkManifestError::InvalidInput(i.to_owned()))
+        })
+        .partition(|res| res.is_ok());
+    if !errs.is_empty() {
+        let errs: Vec<String> = errs
+            .into_iter()
+            .map(|r| format!("Error: {:?}\n", r.unwrap_err()))
+            .collect();
+        return Err(ZkManifestError::InvalidInputs {
+            manifest_path: manifest_path_str,
+            errs,
+        }
+        .into());
     }
 
-    let inputs = meta
-        .unwrap()
-        .as_table()
-        .and_then(|m| m.get("zkprogram"))
-        .and_then(|m| m.as_table())
-        .and_then(|m| m.get("input_order"))
-        .and_then(|m| m.as_array())
-        .ok_or(std::io::Error::new(
-            std::io::ErrorKind::Other,
-            "Invalid Cargo.toml, missing zkprogram metadata",
+    Ok(input_order.into_iter().map(Result::unwrap).collect())
+}
+
+fn parse_cargo_manifest(image_path: &Path) -> Result<(String, Vec<String>)> {
+    let cargo_manifest_path = image_path.join(CARGO_TOML);
+    let cargo_manifest_path_str = cargo_manifest_path.to_string_lossy().to_string();
+    if !cargo_manifest_path.exists() {
+        return Err(
+            ZkManifestError::MissingManifest(image_path.to_string_lossy().to_string()).into(),
+        );
+    }
+    let cargo_manifest = cargo_toml::Manifest::from_path(&cargo_manifest_path).map_err(|err| {
+        ZkManifestError::FailedToLoadManifest {
+            manifest_path: cargo_manifest_path_str.clone(),
+            err,
+        }
+    })?;
+    let cargo_package_name = cargo_manifest
+        .package
+        .as_ref()
+        .map(|p| p.name.clone())
+        .ok_or(ZkManifestError::MissingPackageName(
+            cargo_manifest_path_str.clone(),
         ))?;
+    let input_order = parse_cargo_manifest_inputs(&cargo_manifest, cargo_manifest_path_str)?;
+
+    Ok((cargo_package_name, input_order))
+}
+
+fn build_zkprogram_manifest(
+    image_path: &Path,
+    keypair: &impl Signer,
+    cargo_package_name: String,
+    input_order: Vec<String>,
+) -> Result<ZkProgramManifest> {
+    const RISCV_DOCKER_PATH: &str = "target/riscv-guest/riscv32im-risc0-zkvm-elf/docker";
+    const CARGO_RISCZERO_BUILD_ARGS: &[&str; 4] =
+        &["risczero", "build", "--manifest-path", "Cargo.toml"];
 
     let binary_path = image_path
-        .join("target/riscv-guest/riscv32im-risc0-zkvm-elf/docker")
-        .join(package)
-        .join(package);
-    let output = Command::new("cargo")
+        .join(RISCV_DOCKER_PATH)
+        .join(&cargo_package_name)
+        .join(&cargo_package_name);
+    let output = Command::new(CARGO_COMMAND)
         .current_dir(image_path)
-        .arg("risczero")
-        .arg("build")
-        .arg("--manifest-path")
-        .arg("Cargo.toml")
-        .env("CARGO_TARGET_DIR", image_path.join("target"))
+        .args(CARGO_RISCZERO_BUILD_ARGS)
+        .env("CARGO_TARGET_DIR", image_path.join(TARGET_DIR))
         .output()?;
 
     if output.status.success() {
         let elf_contents = fs::read(&binary_path)?;
-        let image_id = compute_image_id(&elf_contents)
-            .map_err(|_| std::io::Error::new(std::io::ErrorKind::Other, "Invalid image"))?;
+        let image_id = compute_image_id(&elf_contents).map_err(|err| {
+            BonsolCliError::FailedToComputeImageId {
+                binary_path: binary_path.to_string_lossy().to_string(),
+                err,
+            }
+        })?;
         let signature = keypair.sign_message(elf_contents.as_slice());
-        let manifest = ZkProgramManifest {
-            name: package.to_string(),
-            binary_path: binary_path.to_str().unwrap().to_string(),
-            input_order: inputs
-                .iter()
-                .map(|i| i.as_str().unwrap().to_string())
-                .collect(),
+        let zkprogram_manifest = ZkProgramManifest {
+            name: cargo_package_name,
+            binary_path: binary_path
+                .to_str()
+                .ok_or(ZkManifestError::InvalidBinaryPath)?
+                .to_string(),
+            input_order,
             image_id: image_id.to_string(),
             size: elf_contents.len() as u64,
             signature: signature.to_string(),
         };
-        Ok(manifest)
-    } else {
-        let error = String::from_utf8_lossy(&output.stderr);
-        println!("Build failed: {}", error);
-        Err(std::io::Error::new(
-            std::io::ErrorKind::Other,
-            "Build failed",
-        ))
+        return Ok(zkprogram_manifest);
     }
+
+    Err(BonsolCliError::BuildFailure(String::from_utf8_lossy(&output.stderr).to_string()).into())
 }