Repository: tangle-network/gadget (language: rust)

Issue:
[TASK] Contracts not found during deploy are silently ignored
In `deploy_contracts_to_tangle`, if a contract path doesn't exist, it'll be silently ignored:

https://github.com/tangle-network/gadget/blob/726181454e753d36b27d92714453bfcf18f76848/cli/src/deploy.rs#L172-L191

This is problematic, as if one makes a typo in the name of their blueprint manager, it won't get deployed. It should just be a hard error when a path doesn't exist.


Discussion / hints:

Fix patch:
diff --git a/cli/src/deploy.rs b/cli/src/deploy.rs
index aceecbf7b..ed5962c49 100644
--- a/cli/src/deploy.rs
+++ b/cli/src/deploy.rs
@@ -1,7 +1,7 @@
 use alloy_provider::network::TransactionBuilder;
 use alloy_provider::{Provider, WsConnect};
 pub use alloy_signer_local::PrivateKeySigner;
-use color_eyre::eyre::{self, Context, ContextCompat, OptionExt, Result};
+use color_eyre::eyre::{self, Context, ContextCompat, Result};
 use gadget_blueprint_proc_macro_core::{BlueprintManager, ServiceBlueprint};
 use gadget_sdk::clients::tangle::runtime::TangleConfig;
 #[cfg(test)]
@@ -44,6 +44,25 @@ impl Debug for Opts {
     }
 }
 
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+    #[error("Unsupported blueprint manager kind")]
+    UnsupportedBlueprintManager,
+    #[error("Contract not found at `{0}`, check the manager in your `Cargo.toml`!")]
+    ContractNotFound(PathBuf),
+    #[error("Failed to deserialize contract `{0}`: {1}")]
+    DeserializeContract(String, serde_json::Error),
+    #[error("The source at index {0} does not have a valid fetcher")]
+    MissingFetcher(usize),
+    #[error("No matching packages found in the workspace")]
+    NoPackageFound,
+    #[error("The workspace has multiple packages, please specify the package to deploy")]
+    ManyPackages,
+
+    #[error("{0}")]
+    Io(#[from] std::io::Error),
+}
+
 pub async fn generate_service_blueprint<P: Into<PathBuf>, T: AsRef<str>>(
     manifest_metadata_path: P,
     pkg_name: Option<&String>,
@@ -59,8 +78,7 @@ pub async fn generate_service_blueprint<P: Into<PathBuf>, T: AsRef<str>>(
 
     let package = find_package(&metadata, pkg_name)?.clone();
     let package_clone = &package.clone();
-    let mut blueprint =
-        tokio::task::spawn_blocking(move || load_blueprint_metadata(&package)).await??;
+    let mut blueprint = load_blueprint_metadata(&package)?;
     build_contracts_if_needed(package_clone, &blueprint).context("Building contracts")?;
     deploy_contracts_to_tangle(rpc_url.as_ref(), package_clone, &mut blueprint, signer_evm).await?;
 
@@ -158,10 +176,7 @@ async fn deploy_contracts_to_tangle(
     }
     let contract_paths = match blueprint.manager {
         BlueprintManager::Evm(ref path) => vec![(ContractKind::Manager, path)],
-        _ => {
-            eprintln!("Unsupported blueprint manager kind");
-            vec![]
-        }
+        _ => return Err(Error::UnsupportedBlueprintManager.into()),
     };
 
     let abs_contract_paths: Vec<_> = contract_paths
@@ -169,22 +184,32 @@ async fn deploy_contracts_to_tangle(
         .map(|(kind, path)| (kind, resolve_path_relative_to_package(package, path)))
         .collect();
 
-    let contracts = abs_contract_paths
-        .iter()
-        .flat_map(|(kind, path)| {
-            std::fs::read_to_string(path).map(|content| {
-                (
-                    kind,
-                    path.file_stem().unwrap_or_default().to_string_lossy(),
-                    content,
-                )
-            })
-        })
-        .flat_map(|(kind, contract_name, json)| {
-            serde_json::from_str::<alloy_json_abi::ContractObject>(&json)
-                .map(|contract| (kind, contract_name, contract))
-        })
-        .collect::<Vec<_>>();
+    let mut contracts_raw = Vec::new();
+    for (kind, path) in abs_contract_paths {
+        if !path.exists() {
+            return Err(Error::ContractNotFound(path).into());
+        }
+
+        let content = std::fs::read_to_string(&path)?;
+        contracts_raw.push((
+            kind,
+            path.file_stem()
+                .unwrap_or_default()
+                .to_string_lossy()
+                .into_owned(),
+            content,
+        ));
+    }
+
+    let mut contracts = Vec::new();
+    for (kind, contract_name, json) in contracts_raw {
+        let contract = match serde_json::from_str::<alloy_json_abi::ContractObject>(&json) {
+            Ok(contract) => contract,
+            Err(e) => return Err(Error::DeserializeContract(contract_name, e).into()),
+        };
+
+        contracts.push((kind, contract_name, contract));
+    }
 
     if contracts.is_empty() {
         return Ok(());
@@ -242,10 +267,7 @@ fn build_contracts_if_needed(
 ) -> Result<()> {
     let pathes_to_check = match blueprint.manager {
         BlueprintManager::Evm(ref path) => vec![path],
-        _ => {
-            eprintln!("Unsupported blueprint manager kind");
-            vec![]
-        }
+        _ => return Err(Error::UnsupportedBlueprintManager.into()),
     };
 
     let abs_pathes_to_check: Vec<_> = pathes_to_check
@@ -301,29 +323,29 @@ fn bake_blueprint(
     // The source includes where to fetch the Blueprint, the name of the blueprint, and
     // the name of the binary. From this data, we create the blueprint's [`ServiceBlueprint`]
     for (idx, source) in sources.iter_mut().enumerate() {
-        if let Some(fetchers) = source["fetcher"].as_object_mut() {
-            let fetcher_fields = fetchers
-                .iter_mut()
-                .next()
-                .expect("Should be at least one fetcher")
-                .1;
-            for (_key, value) in fetcher_fields
-                .as_object_mut()
-                .expect("Fetcher should be a map")
-            {
-                if value.is_array() {
-                    let xs = value.as_array_mut().expect("Value should be an array");
-                    for x in xs {
-                        if x.is_object() {
-                            convert_to_bytes_or_null(&mut x["name"]);
-                        }
+        let Some(fetchers) = source["fetcher"].as_object_mut() else {
+            return Err(Error::MissingFetcher(idx).into());
+        };
+
+        let fetcher_fields = fetchers
+            .iter_mut()
+            .next()
+            .expect("Should be at least one fetcher")
+            .1;
+        for (_key, value) in fetcher_fields
+            .as_object_mut()
+            .expect("Fetcher should be a map")
+        {
+            if value.is_array() {
+                let xs = value.as_array_mut().expect("Value should be an array");
+                for x in xs {
+                    if x.is_object() {
+                        convert_to_bytes_or_null(&mut x["name"]);
                     }
-                } else {
-                    convert_to_bytes_or_null(value);
                 }
+            } else {
+                convert_to_bytes_or_null(value);
             }
-        } else {
-            panic!("The source at index {idx} does not have a valid fetcher");
         }
     }
 
@@ -365,26 +387,24 @@ fn find_package<'m>(
     pkg_name: Option<&String>,
 ) -> Result<&'m cargo_metadata::Package, eyre::Error> {
     match metadata.workspace_members.len() {
-        0 => Err(eyre::eyre!("No packages found in the workspace")),
+        0 => Err(Error::NoPackageFound.into()),
         1 => metadata
             .packages
             .iter()
             .find(|p| p.id == metadata.workspace_members[0])
-            .ok_or_eyre("No package found in the workspace"),
+            .ok_or(Error::NoPackageFound.into()),
         _more_than_one if pkg_name.is_some() => metadata
             .packages
             .iter()
             .find(|p| pkg_name.is_some_and(|v| &p.name == v))
-            .ok_or_eyre("No package found in the workspace with the specified name"),
+            .ok_or(Error::NoPackageFound.into()),
         _otherwise => {
             eprintln!("Please specify the package to deploy:");
             for package in metadata.packages.iter() {
                 eprintln!("Found: {}", package.name);
             }
             eprintln!();
-            Err(eyre::eyre!(
-                "The workspace has multiple packages, please specify the package to deploy"
-            ))
+            Err(Error::ManyPackages.into())
         }
     }
 }
diff --git a/cli/src/foundry/forge.rs b/cli/src/foundry/forge.rs
index 5ef77adf2..de006682f 100644
--- a/cli/src/foundry/forge.rs
+++ b/cli/src/foundry/forge.rs
@@ -1,8 +1,16 @@
 use super::CommandInstalled;
-use color_eyre::{eyre::eyre, Result};
+use color_eyre::Result;
 use gadget_sdk::tracing;
 use std::process::Command;
 
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+    #[error("Failed to build contracts")]
+    BuildContracts,
+    #[error("Failed to install dependencies")]
+    InstallDependencies,
+}
+
 pub struct Forge {
     cmd: Command,
 }
@@ -42,7 +50,7 @@ impl Forge {
             .spawn()?
             .wait()?;
         if !status.success() {
-            return Err(eyre!("Failed to build contracts"));
+            return Err(Error::BuildContracts.into());
         }
         Ok(())
     }
@@ -56,7 +64,7 @@ impl Forge {
             .stdout(std::process::Stdio::inherit())
             .output()?;
         if !output.status.success() {
-            return Err(eyre!("Failed to install dependencies"));
+            return Err(Error::InstallDependencies.into());
         }
         Ok(())
     }
diff --git a/cli/src/keys.rs b/cli/src/keys.rs
index ae5854201..1e4f111d6 100644
--- a/cli/src/keys.rs
+++ b/cli/src/keys.rs
@@ -10,6 +10,12 @@ use std::path::PathBuf;
 use std::str::FromStr;
 use w3f_bls::serialize::SerializableToBytes;
 
+#[derive(thiserror::Error, Debug)]
+pub enum Error {
+    #[error("Unknown key type: {0}")]
+    UnknownKeyType(String),
+}
+
 #[derive(Clone, Debug)]
 pub enum KeyType {
     Sr25519,
@@ -44,16 +50,17 @@ impl ValueEnum for KeyType {
 }
 
 impl FromStr for KeyType {
-    type Err = String;
+    type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        match s.to_lowercase().as_str() {
+        let lower = s.to_lowercase();
+        match lower.as_str() {
             "sr25519" => Ok(Self::Sr25519),
             "ed25519" => Ok(Self::Ed25519),
             "ecdsa" => Ok(Self::Ecdsa),
             "bls381" => Ok(Self::Bls381),
             "blsbn254" => Ok(Self::BlsBn254),
-            _ => Err(format!("Unknown key type: {}", s)),
+            _ => Err(Error::UnknownKeyType(lower)),
         }
     }
 }
@@ -68,20 +75,15 @@ pub fn generate_key(
         None => GenericKeyStore::Mem(InMemoryKeystore::new()),
         Some(file_path) => {
             // Filesystem Keystore
-            GenericKeyStore::Fs(
-                FilesystemKeystore::open(file_path).map_err(|e| eyre!(e.to_string()))?,
-            )
+            GenericKeyStore::Fs(FilesystemKeystore::open(file_path)?)
         }
     };
 
     let (public, secret) = match key_type {
         KeyType::Sr25519 => {
-            let public_key = keystore
-                .sr25519_generate_new(seed)
-                .map_err(|e| eyre!(e.to_string()))?;
+            let public_key = keystore.sr25519_generate_new(seed)?;
             let secret = keystore
-                .expose_sr25519_secret(&public_key)
-                .map_err(|e| eyre!(e.to_string()))?
+                .expose_sr25519_secret(&public_key)?
                 .ok_or(eyre!("Failed to expose secret"))?;
             (
                 hex::encode(public_key.to_bytes()),
@@ -89,22 +91,16 @@ pub fn generate_key(
             )
         }
         KeyType::Ed25519 => {
-            let public_key = keystore
-                .ed25519_generate_new(seed)
-                .map_err(|e| eyre!(e.to_string()))?;
+            let public_key = keystore.ed25519_generate_new(seed)?;
             let secret = keystore
-                .expose_ed25519_secret(&public_key)
-                .map_err(|e| eyre!(e.to_string()))?
+                .expose_ed25519_secret(&public_key)?
                 .ok_or(eyre!("Failed to expose secret"))?;
             (hex::encode(public_key), hex::encode(secret))
         }
         KeyType::Ecdsa => {
-            let public_key = keystore
-                .ecdsa_generate_new(seed)
-                .map_err(|e| eyre!(e.to_string()))?;
+            let public_key = keystore.ecdsa_generate_new(seed)?;
             let secret = keystore
-                .expose_ecdsa_secret(&public_key)
-                .map_err(|e| eyre!(e.to_string()))?
+                .expose_ecdsa_secret(&public_key)?
                 .ok_or(eyre!("Failed to expose secret"))?;
             (
                 hex::encode(public_key.to_sec1_bytes()),
@@ -112,12 +108,9 @@ pub fn generate_key(
             )
         }
         KeyType::Bls381 => {
-            let public_key = keystore
-                .bls381_generate_new(seed)
-                .map_err(|e| eyre!(e.to_string()))?;
+            let public_key = keystore.bls381_generate_new(seed)?;
             let secret = keystore
-                .expose_bls381_secret(&public_key)
-                .map_err(|e| eyre!(e.to_string()))?
+                .expose_bls381_secret(&public_key)?
                 .ok_or(eyre!("Failed to expose secret"))?;
             (
                 hex::encode(public_key.0.to_string()),
@@ -125,12 +118,9 @@ pub fn generate_key(
             )
         }
         KeyType::BlsBn254 => {
-            let public_key = keystore
-                .bls_bn254_generate_new(seed)
-                .map_err(|e| eyre!(e.to_string()))?;
+            let public_key = keystore.bls_bn254_generate_new(seed)?;
             let secret = keystore
-                .expose_bls_bn254_secret(&public_key)
-                .map_err(|e| eyre!(e.to_string()))?
+                .expose_bls_bn254_secret(&public_key)?
                 .ok_or(eyre!("Failed to expose secret"))?;
             (public_key.g1().to_string(), secret.0.to_string())
         }


Test patch: