Repository: CosmWasm/cosmwasm
Pull request #1039 (2021-08-03 14:18:00)

Improve export scanning to expose required features
There has been a request by wasmd team to be able to know which features a contract requires. This is not strictly needed for compatibility checks because those checks are done in cosmwasm-vm. But it allows to collect metedata of uploaded contracts for business analysis purposes. It allows chains to see which features are actually used (or at least required). It also allows block explorers to show this information based on events emitted by the upload transaction. This is a preparation for a post-1.0 world where the interface version is fixed but new optional features can be requested by the contract.

It contains a nice little refactoring. Instead of duplicating code for parity_wasm (used pre-instantiation for static analysis) and wasmer (used on instantiation), a new tait `ExportInfo` makes the shared functionality available from both sources.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 96e20d5d8..1c7bf1c14 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -66,6 +66,10 @@ and this project adheres to
   downstream code).
 - cosmwasm-std: `Event::attr` has been renamed to `Event::add_attribute` for
   consistency with other types like `Response`.
+- cosmwasm-vm: `Instance::required_features` changed from a property to a getter
+  method.
+- cosmwasm-vm: Add `required_features` field to `AnalysisReport` which is
+  returned by `Cache::analyze`.
 
 [#995]: https://github.com/CosmWasm/cosmwasm/pull/995
 
diff --git a/packages/vm/src/cache.rs b/packages/vm/src/cache.rs
index da9ffac06..81d27a62b 100644
--- a/packages/vm/src/cache.rs
+++ b/packages/vm/src/cache.rs
@@ -9,6 +9,7 @@ use crate::backend::{Backend, BackendApi, Querier, Storage};
 use crate::checksum::Checksum;
 use crate::compatibility::check_wasm;
 use crate::errors::{VmError, VmResult};
+use crate::features::required_features_from_module;
 use crate::instance::{Instance, InstanceOptions};
 use crate::modules::{FileSystemCache, InMemoryCache, PinnedMemoryCache};
 use crate::size::Size;
@@ -72,8 +73,10 @@ pub struct Cache<A: BackendApi, S: Storage, Q: Querier> {
     type_querier: PhantomData<Q>,
 }
 
+#[derive(PartialEq, Debug)]
 pub struct AnalysisReport {
     pub has_ibc_entry_points: bool,
+    pub required_features: HashSet<String>,
 }
 
 impl<A, S, Q> Cache<A, S, Q>
@@ -185,6 +188,7 @@ where
         let module = deserialize_wasm(&wasm)?;
         Ok(AnalysisReport {
             has_ibc_entry_points: has_ibc_entry_points(&module),
+            required_features: required_features_from_module(&module),
         })
     }
 
@@ -355,6 +359,7 @@ mod tests {
     use cosmwasm_std::{coins, Empty};
     use std::fs::OpenOptions;
     use std::io::Write;
+    use std::iter::FromIterator;
     use tempfile::TempDir;
 
     const TESTING_GAS_LIMIT: u64 = 4_000_000;
@@ -366,6 +371,7 @@ mod tests {
     const TESTING_MEMORY_CACHE_SIZE: Size = Size::mebi(200);
 
     static CONTRACT: &[u8] = include_bytes!("../testdata/hackatom.wasm");
+    static IBC_CONTRACT: &[u8] = include_bytes!("../testdata/ibc_reflect.wasm");
 
     fn default_features() -> HashSet<String> {
         features_from_csv("staking")
@@ -380,6 +386,15 @@ mod tests {
         }
     }
 
+    fn make_stargate_testing_options() -> CacheOptions {
+        CacheOptions {
+            base_dir: TempDir::new().unwrap().into_path(),
+            supported_features: features_from_csv("staking,stargate"),
+            memory_cache_size: TESTING_MEMORY_CACHE_SIZE,
+            instance_memory_limit: TESTING_MEMORY_LIMIT,
+        }
+    }
+
     #[test]
     fn save_wasm_works() {
         let cache: Cache<MockApi, MockStorage, MockQuerier> =
@@ -943,6 +958,35 @@ mod tests {
         assert_eq!(code, loaded);
     }
 
+    #[test]
+    fn analyze_works() {
+        let cache: Cache<MockApi, MockStorage, MockQuerier> =
+            unsafe { Cache::new(make_stargate_testing_options()).unwrap() };
+
+        let checksum1 = cache.save_wasm(CONTRACT).unwrap();
+        let report1 = cache.analyze(&checksum1).unwrap();
+        assert_eq!(
+            report1,
+            AnalysisReport {
+                has_ibc_entry_points: false,
+                required_features: HashSet::new(),
+            }
+        );
+
+        let checksum2 = cache.save_wasm(IBC_CONTRACT).unwrap();
+        let report2 = cache.analyze(&checksum2).unwrap();
+        assert_eq!(
+            report2,
+            AnalysisReport {
+                has_ibc_entry_points: true,
+                required_features: HashSet::from_iter(vec![
+                    "staking".to_string(),
+                    "stargate".to_string()
+                ]),
+            }
+        );
+    }
+
     #[test]
     fn pin_unpin_works() {
         let cache = unsafe { Cache::new(make_testing_options()).unwrap() };
diff --git a/packages/vm/src/compatibility.rs b/packages/vm/src/compatibility.rs
index dcacfda0d..0318252dc 100644
--- a/packages/vm/src/compatibility.rs
+++ b/packages/vm/src/compatibility.rs
@@ -5,7 +5,7 @@ use std::collections::HashSet;
 use crate::errors::{VmError, VmResult};
 use crate::features::required_features_from_module;
 use crate::limited::LimitedDisplay;
-use crate::static_analysis::{deserialize_wasm, exported_functions};
+use crate::static_analysis::{deserialize_wasm, ExportInfo};
 
 /// Lists all imports we provide upon instantiating the instance in Instance::from_module()
 /// This should be updated when new imports are added
@@ -87,7 +87,7 @@ fn check_wasm_memories(module: &Module) -> VmResult<()> {
 }
 
 fn check_wasm_exports(module: &Module) -> VmResult<()> {
-    let available_exports: HashSet<String> = exported_functions(module);
+    let available_exports: HashSet<String> = module.exported_function_names(None);
     for required_export in REQUIRED_EXPORTS {
         if !available_exports.contains(*required_export) {
             return Err(VmError::static_validation_err(format!(
diff --git a/packages/vm/src/features.rs b/packages/vm/src/features.rs
index daeb9fffa..4de16d0c9 100644
--- a/packages/vm/src/features.rs
+++ b/packages/vm/src/features.rs
@@ -1,6 +1,6 @@
-use parity_wasm::elements::{Internal, Module};
 use std::collections::HashSet;
-use wasmer::{ExternType, Instance as WasmerInstance};
+
+use crate::static_analysis::ExportInfo;
 
 const REQUIRES_PREFIX: &str = "requires_";
 
@@ -13,49 +13,27 @@ pub fn features_from_csv(csv: &str) -> HashSet<String> {
         .collect()
 }
 
-pub fn required_features_from_wasmer_instance(wasmer_instance: &WasmerInstance) -> HashSet<String> {
-    let module = wasmer_instance.module();
+/// Implementation for check_wasm, based on static analysis of the bytecode.
+/// This is used for code upload, to perform check before compiling the Wasm.
+pub fn required_features_from_module(module: &impl ExportInfo) -> HashSet<String> {
     module
-        .exports()
-        .filter_map(|export| {
-            if let ExternType::Function { .. } = export.ty() {
-                let name = export.name();
-                if name.starts_with(REQUIRES_PREFIX) && name.len() > REQUIRES_PREFIX.len() {
-                    let required_feature = name.to_string().split_off(REQUIRES_PREFIX.len());
-                    return Some(required_feature);
-                }
+        .exported_function_names(Some(REQUIRES_PREFIX))
+        .into_iter()
+        .filter_map(|name| {
+            if name.len() > REQUIRES_PREFIX.len() {
+                let (_, required_feature) = name.split_at(REQUIRES_PREFIX.len());
+                Some(required_feature.to_string())
+            } else {
+                None
             }
-            None
         })
         .collect()
 }
 
-/// Implementation for check_wasm, based on static analysis of the bytecode.
-/// This is used for code upload, to perform check before compiling the Wasm.
-pub fn required_features_from_module(module: &Module) -> HashSet<String> {
-    match module.export_section() {
-        None => HashSet::new(),
-        Some(export_section) => export_section
-            .entries()
-            .iter()
-            .filter_map(|entry| {
-                if let Internal::Function(_) = entry.internal() {
-                    let name = entry.field();
-                    if name.starts_with(REQUIRES_PREFIX) && name.len() > REQUIRES_PREFIX.len() {
-                        let (_, required_feature) = name.split_at(REQUIRES_PREFIX.len());
-                        return Some(required_feature.to_string());
-                    }
-                }
-                None
-            })
-            .collect(),
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
-    use parity_wasm::elements::deserialize_buffer;
+    use crate::static_analysis::deserialize_wasm;
 
     #[test]
     fn features_from_csv_works() {
@@ -95,7 +73,7 @@ mod tests {
             )"#,
         )
         .unwrap();
-        let module = deserialize_buffer(&wasm).unwrap();
+        let module = deserialize_wasm(&wasm).unwrap();
 
         let required_features = required_features_from_module(&module);
         assert_eq!(required_features.len(), 3);
@@ -107,7 +85,7 @@ mod tests {
     #[test]
     fn required_features_from_module_works_without_exports_section() {
         let wasm = wat::parse_str(r#"(module)"#).unwrap();
-        let module = deserialize_buffer(&wasm).unwrap();
+        let module = deserialize_wasm(&wasm).unwrap();
         let required_features = required_features_from_module(&module);
         assert_eq!(required_features.len(), 0);
     }
diff --git a/packages/vm/src/instance.rs b/packages/vm/src/instance.rs
index b1ec320a6..39a89ca1d 100644
--- a/packages/vm/src/instance.rs
+++ b/packages/vm/src/instance.rs
@@ -7,7 +7,7 @@ use crate::backend::{Backend, BackendApi, Querier, Storage};
 use crate::conversion::{ref_to_u32, to_u32};
 use crate::environment::Environment;
 use crate::errors::{CommunicationError, VmError, VmResult};
-use crate::features::required_features_from_wasmer_instance;
+use crate::features::required_features_from_module;
 use crate::imports::{
     do_addr_canonicalize, do_addr_humanize, do_addr_validate, do_db_read, do_db_remove,
     do_db_write, do_debug, do_ed25519_batch_verify, do_ed25519_verify, do_query_chain,
@@ -46,7 +46,6 @@ pub struct Instance<A: BackendApi, S: Storage, Q: Querier> {
     /// This instance should only be accessed via the Environment, which provides safe access.
     _inner: Box<WasmerInstance>,
     env: Environment<A, S, Q>,
-    pub required_features: HashSet<String>,
 }
 
 impl<A, S, Q> Instance<A, S, Q>
@@ -207,7 +206,6 @@ where
             },
         )?);
 
-        let required_features = required_features_from_wasmer_instance(wasmer_instance.as_ref());
         let instance_ptr = NonNull::from(wasmer_instance.as_ref());
         env.set_wasmer_instance(Some(instance_ptr));
         env.set_gas_left(gas_limit);
@@ -215,7 +213,6 @@ where
         let instance = Instance {
             _inner: wasmer_instance,
             env,
-            required_features,
         };
         Ok(instance)
     }
@@ -239,6 +236,15 @@ where
         }
     }
 
+    /// Returns the features required by this contract.
+    ///
+    /// This is not needed for production because we can do static analysis
+    /// on the Wasm file before instatiation to obtain this information. It's
+    /// only kept because it can be handy for integration testing.
+    pub fn required_features(&self) -> HashSet<String> {
+        required_features_from_module(self._inner.module())
+    }
+
     /// Returns the size of the default memory in pages.
     /// This provides a rough idea of the peak memory consumption. Note that
     /// Wasm memory always grows in 64 KiB steps (pages) and can never shrink
@@ -357,7 +363,7 @@ mod tests {
         let (instance_options, memory_limit) = mock_instance_options();
         let instance =
             Instance::from_code(CONTRACT, backend, instance_options, memory_limit).unwrap();
-        assert_eq!(instance.required_features.len(), 0);
+        assert_eq!(instance.required_features().len(), 0);
     }
 
     #[test]
@@ -379,10 +385,10 @@ mod tests {
         let backend = mock_backend(&[]);
         let (instance_options, memory_limit) = mock_instance_options();
         let instance = Instance::from_code(&wasm, backend, instance_options, memory_limit).unwrap();
-        assert_eq!(instance.required_features.len(), 3);
-        assert!(instance.required_features.contains("nutrients"));
-        assert!(instance.required_features.contains("sun"));
-        assert!(instance.required_features.contains("water"));
+        assert_eq!(instance.required_features().len(), 3);
+        assert!(instance.required_features().contains("nutrients"));
+        assert!(instance.required_features().contains("sun"));
+        assert!(instance.required_features().contains("water"));
     }
 
     #[test]
diff --git a/packages/vm/src/static_analysis.rs b/packages/vm/src/static_analysis.rs
index 085a5d683..748d6cbd1 100644
--- a/packages/vm/src/static_analysis.rs
+++ b/packages/vm/src/static_analysis.rs
@@ -21,26 +21,62 @@ pub fn deserialize_wasm(wasm_code: &[u8]) -> VmResult<Module> {
     })
 }
 
-pub fn exported_functions(module: &Module) -> HashSet<String> {
-    module
-        .export_section()
-        .map_or(HashSet::default(), |export_section| {
-            export_section
-                .entries()
-                .iter()
-                .filter_map(|entry| match entry.internal() {
-                    Internal::Function(_) => Some(entry.field().to_string()),
-                    _ => None,
-                })
-                .collect()
-        })
+/// A trait that allows accessing shared functionality of `parity_wasm::elements::Module`
+/// and `wasmer::Module` in a shared fashion.
+pub trait ExportInfo {
+    /// Returns all exported function names with the given prefix
+    fn exported_function_names(&self, prefix: Option<&str>) -> HashSet<String>;
+}
+
+impl ExportInfo for Module {
+    fn exported_function_names(&self, prefix: Option<&str>) -> HashSet<String> {
+        self.export_section()
+            .map_or(HashSet::default(), |export_section| {
+                export_section
+                    .entries()
+                    .iter()
+                    .filter_map(|entry| match entry.internal() {
+                        Internal::Function(_) => Some(entry.field()),
+                        _ => None,
+                    })
+                    .filter(|name| {
+                        if let Some(required_prefix) = prefix {
+                            name.starts_with(required_prefix)
+                        } else {
+                            true
+                        }
+                    })
+                    .map(|name| name.to_string())
+                    .collect()
+            })
+    }
+}
+
+impl ExportInfo for wasmer::Module {
+    fn exported_function_names(&self, prefix: Option<&str>) -> HashSet<String> {
+        self.exports()
+            .functions()
+            .filter_map(|function_export| {
+                let name = function_export.name();
+                if let Some(required_prefix) = prefix {
+                    if name.starts_with(required_prefix) {
+                        Some(name.to_string())
+                    } else {
+                        None
+                    }
+                } else {
+                    Some(name.to_string())
+                }
+            })
+            .collect()
+    }
 }
 
 /// Returns true if and only if all IBC entry points ([`REQUIRED_IBC_EXPORTS`])
 /// exist as exported functions. This does not guarantee the entry points
 /// are functional and for simplicity does not even check their signatures.
-pub fn has_ibc_entry_points(module: &Module) -> bool {
-    let available_exports = exported_functions(module);
+pub fn has_ibc_entry_points(module: &impl ExportInfo) -> bool {
+    let available_exports = module.exported_function_names(None);
     REQUIRED_IBC_EXPORTS
         .iter()
         .all(|required| available_exports.contains(*required))
@@ -51,6 +87,7 @@ mod tests {
     use super::*;
     use parity_wasm::elements::Internal;
     use std::iter::FromIterator;
+    use wasmer::{Cranelift, Store, Universal};
 
     static CONTRACT: &[u8] = include_bytes!("../testdata/hackatom.wasm");
     static CORRUPTED: &[u8] = include_bytes!("../testdata/corrupted.wasm");
@@ -88,10 +125,37 @@ mod tests {
     }
 
     #[test]
-    fn exported_functions_works() {
+    fn exported_function_names_works_for_parity_with_no_prefix() {
+        let wasm = wat::parse_str(r#"(module)"#).unwrap();
+        let module = deserialize_wasm(&wasm).unwrap();
+        let exports = module.exported_function_names(None);
+        assert_eq!(exports, HashSet::new());
+
+        let wasm = wat::parse_str(
+            r#"(module
+                (memory 3)
+                (export "memory" (memory 0))
+
+                (type (func))
+                (func (type 0) nop)
+                (export "foo" (func 0))
+                (export "bar" (func 0))
+            )"#,
+        )
+        .unwrap();
+        let module = deserialize_wasm(&wasm).unwrap();
+        let exports = module.exported_function_names(None);
+        assert_eq!(
+            exports,
+            HashSet::from_iter(vec!["foo".to_string(), "bar".to_string()])
+        );
+    }
+
+    #[test]
+    fn exported_function_names_works_for_parity_with_prefix() {
         let wasm = wat::parse_str(r#"(module)"#).unwrap();
         let module = deserialize_wasm(&wasm).unwrap();
-        let exports = exported_functions(&module);
+        let exports = module.exported_function_names(Some("b"));
         assert_eq!(exports, HashSet::new());
 
         let wasm = wat::parse_str(
@@ -103,14 +167,74 @@ mod tests {
                 (func (type 0) nop)
                 (export "foo" (func 0))
                 (export "bar" (func 0))
+                (export "baz" (func 0))
             )"#,
         )
         .unwrap();
         let module = deserialize_wasm(&wasm).unwrap();
-        let exports = exported_functions(&module);
+        let exports = module.exported_function_names(Some("b"));
+        assert_eq!(
+            exports,
+            HashSet::from_iter(vec!["bar".to_string(), "baz".to_string()])
+        );
+    }
+
+    #[test]
+    fn exported_function_names_works_for_wasmer_with_no_prefix() {
+        let wasm = wat::parse_str(r#"(module)"#).unwrap();
+        let store = Store::new(&Universal::new(Cranelift::default()).engine());
+        let module = wasmer::Module::new(&store, wasm).unwrap();
+        let exports = module.exported_function_names(None);
+        assert_eq!(exports, HashSet::new());
+
+        let wasm = wat::parse_str(
+            r#"(module
+                (memory 3)
+                (export "memory" (memory 0))
+
+                (type (func))
+                (func (type 0) nop)
+                (export "foo" (func 0))
+                (export "bar" (func 0))
+            )"#,
+        )
+        .unwrap();
+        let store = Store::new(&Universal::new(Cranelift::default()).engine());
+        let module = wasmer::Module::new(&store, wasm).unwrap();
+        let exports = module.exported_function_names(None);
+        assert_eq!(
+            exports,
+            HashSet::from_iter(vec!["foo".to_string(), "bar".to_string()])
+        );
+    }
+
+    #[test]
+    fn exported_function_names_works_for_wasmer_with_prefix() {
+        let wasm = wat::parse_str(r#"(module)"#).unwrap();
+        let store = Store::new(&Universal::new(Cranelift::default()).engine());
+        let module = wasmer::Module::new(&store, wasm).unwrap();
+        let exports = module.exported_function_names(Some("b"));
+        assert_eq!(exports, HashSet::new());
+
+        let wasm = wat::parse_str(
+            r#"(module
+                (memory 3)
+                (export "memory" (memory 0))
+
+                (type (func))
+                (func (type 0) nop)
+                (export "foo" (func 0))
+                (export "bar" (func 0))
+                (export "baz" (func 0))
+            )"#,
+        )
+        .unwrap();
+        let store = Store::new(&Universal::new(Cranelift::default()).engine());
+        let module = wasmer::Module::new(&store, wasm).unwrap();
+        let exports = module.exported_function_names(Some("b"));
         assert_eq!(
             exports,
-            HashSet::from_iter(vec!["foo".to_string(), "bar".to_string(),])
+            HashSet::from_iter(vec!["bar".to_string(), "baz".to_string()])
         );
     }
 


Test patch:
diff --git a/contracts/hackatom/tests/integration.rs b/contracts/hackatom/tests/integration.rs
index 055bcc2a1..7f13e484e 100644
--- a/contracts/hackatom/tests/integration.rs
+++ b/contracts/hackatom/tests/integration.rs
@@ -53,7 +53,7 @@ fn make_init_msg() -> (InstantiateMsg, String) {
 #[test]
 fn proper_initialization() {
     let mut deps = mock_instance(WASM, &[]);
-    assert_eq!(deps.required_features.len(), 0);
+    assert_eq!(deps.required_features().len(), 0);
 
     let verifier = String::from("verifies");
     let beneficiary = String::from("benefits");
diff --git a/contracts/staking/tests/integration.rs b/contracts/staking/tests/integration.rs
index 102eff94f..c638be08f 100644
--- a/contracts/staking/tests/integration.rs
+++ b/contracts/staking/tests/integration.rs
@@ -86,8 +86,8 @@ fn proper_initialization() {
     );
     let (instance_options, memory_limit) = mock_instance_options();
     let mut deps = Instance::from_code(WASM, backend, instance_options, memory_limit).unwrap();
-    assert_eq!(deps.required_features.len(), 1);
-    assert!(deps.required_features.contains("staking"));
+    assert_eq!(deps.required_features().len(), 1);
+    assert!(deps.required_features().contains("staking"));
 
     let creator = String::from("creator");
     let msg = InstantiateMsg {