Prevent silent (ish) failure of agent validation

CHANGELOG-UNRELEASED.md

@@ -8,6 +8,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 * The sim2h switch-board server is now caching if a node is missing data and periodically checks back in. This makes it more resilient against unforseen problems like connection drops which otherwise could only be recovered through an explicit reconnection of the node. [#1834](https://github.com/holochain/holochain-rust/pull/1834) 
 
 ### Changed
+* DNA is now checked for invalid zome artifacts. Validation callbacks that fail unexpectedly will now panic rather than fail validation. `hc package` `--strip-meta` flag is now `--include-meta`. [#1838](https://github.com/holochain/holochain-rust/pull/1838) 
 
 ### Deprecated
 

crates/cli/src/cli/package.rs

@@ -56,15 +56,15 @@ fn hdk_version_compare(hdk_version: &HDKVersion, cargo_toml: &str) -> DefaultRes
 }
 
 struct Packager {
-    strip_meta: bool,
+    include_meta: bool,
 }
 
 impl Packager {
-    fn new(strip_meta: bool) -> Packager {
-        Packager { strip_meta }
+    fn new(include_meta: bool) -> Packager {
+        Packager { include_meta }
     }
 
-    pub fn package(strip_meta: bool, output: PathBuf, properties: Value) -> DefaultResult<()> {
+    pub fn package(include_meta: bool, output: PathBuf, properties: Value) -> DefaultResult<()> {
         // First, check whether they have `cargo` installed, since it will be needed for packaging
         // TODO: in the future, don't check for this here, since other build tools and languages
         // could be used
@@ -81,7 +81,7 @@ impl Packager {
             return Ok(());
         }
 
-        Packager::new(strip_meta).run(&output, properties)
+        Packager::new(include_meta).run(&output, properties)
     }
 
     fn run(&self, output: &PathBuf, mut properties: Value) -> DefaultResult<()> {
@@ -295,7 +295,7 @@ impl Packager {
             }
         }
 
-        if !self.strip_meta {
+        if self.include_meta {
             if !meta_tree.is_empty() {
                 meta_section.insert(META_TREE_SECTION_NAME.into(), meta_tree.into());
             }

crates/cli/src/main.rs

@@ -41,7 +41,10 @@ enum Cli {
     ///  Builds DNA source files into a single .dna.json DNA file
     Package {
         #[structopt(long)]
-        /// Strips all __META__ sections off the target bundle. Makes unpacking of the bundle impossible
+        /// Adds __META__ fields in the dna.json which allow it to be unpacked
+        include_meta: bool,
+        #[structopt(long)]
+        /// Included for backward compatibility. Does nothing as stripping meta is now the default
         strip_meta: bool,
         #[structopt(long, short, parse(from_os_str))]
         output: Option<PathBuf>,
@@ -167,10 +170,12 @@ fn run() -> HolochainResult<()> {
     match args {
         // If using default path, we'll create if necessary; otherwise, target dir must exist
         Cli::Package {
-            strip_meta,
+            include_meta,
             output,
             properties: properties_string,
+            strip_meta,
         } => {
+            let _ = strip_meta;
             let output = if output.is_some() {
                 output.unwrap()
             } else {
@@ -182,9 +187,8 @@ fn run() -> HolochainResult<()> {
                 .unwrap_or_else(|| Ok(json!({})));
 
             match properties {
-                Ok(properties) => {
-                    cli::package(strip_meta, output, properties).map_err(HolochainError::Default)?
-                }
+                Ok(properties) => cli::package(include_meta, output, properties)
+                    .map_err(HolochainError::Default)?,
                 Err(e) => {
                     return Err(HolochainError::Default(format_err!(
                         "Failed to parse properties argument as JSON: {:?}",

crates/conductor_lib/src/conductor/base.rs

@@ -1208,7 +1208,9 @@ impl Conductor {
         let mut f = File::open(file)?;
         let mut contents = String::new();
         f.read_to_string(&mut contents)?;
-        Dna::try_from(JsonString::from_json(&contents)).map_err(|err| err.into())
+        let dna: Dna = Dna::try_from(JsonString::from_json(&contents))?;
+        dna.verify()?;
+        Ok(dna)
     }
 
     /// Default KeyLoader that actually reads files from the filesystem

crates/core/src/nucleus/actions/run_validation_callback.rs

@@ -48,10 +48,12 @@ pub async fn run_validation_callback(
                             String::from("JSON object does not match entry schema").into(),
                         ))
                     } else {
-                        Err(ValidationError::Error(error_string.into()))
+                        // an unknown error from the ribosome should panic rather than
+                        // silently failing validation
+                        panic!(error_string)
                     }
                 }
-                Err(error) => Err(ValidationError::Error(error.to_string().into())),
+                Err(error) => panic!(error.to_string()), // same here
             };
 
             lax_send_sync(

crates/core_types/src/dna/mod.rs

@@ -39,7 +39,7 @@ use crate::{
         fn_declarations::{FnDeclaration, TraitFns},
     },
     entry::entry_type::EntryType,
-    error::{DnaError, HolochainError},
+    error::{DnaError, HcResult, HolochainError},
 };
 
 use holochain_persistence_api::cas::content::{AddressableContent, Content};
@@ -257,6 +257,35 @@ impl Dna {
             .flatten()
             .collect()
     }
+
+    // Check that all the zomes in the DNA have code with the required callbacks
+    // TODO: Add more advanced checks that actually try and call required functions
+    pub fn verify(&self) -> HcResult<()> {
+        let errors: Vec<HolochainError> = self
+            .zomes
+            .iter()
+            .map(|(zome_name, zome)| {
+                // currently just check the zome has some code
+                if zome.code.code.len() > 0 {
+                    Ok(())
+                } else {
+                    Err(HolochainError::ErrorGeneric(format!(
+                        "Zome {} has no code!",
+                        zome_name
+                    )))
+                }
+            })
+            .filter_map(|r| r.err())
+            .collect();
+        if errors.is_empty() {
+            Ok(())
+        } else {
+            Err(HolochainError::ErrorGeneric(format!(
+                "invalid DNA: {:?}",
+                errors
+            )))
+        }
+    }
 }
 
 impl Hash for Dna {
@@ -421,6 +450,28 @@ pub mod tests {
         );
     }
 
+    #[test]
+    fn test_dna_verify() {
+        let dna = test_dna();
+        assert!(dna.verify().is_ok())
+    }
+
+    #[test]
+    fn test_dna_verify_fail() {
+        // should error because code is empty
+        let dna = Dna::try_from(JsonString::from_json(
+            r#"{
+                "zomes": {
+                    "my_zome": {
+                        "code": {"code": ""}
+                    }
+                }
+            }"#,
+        ))
+        .unwrap();
+        assert!(dna.verify().is_err())
+    }
+
     static UNIT_UUID: &'static str = "00000000-0000-0000-0000-000000000000";
 
     fn test_empty_dna() -> Dna {